Feat/shielded outputs#864
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces shielded transaction support to the Hathor mobile wallet by integrating a native iOS cryptographic module via UniFFI, implementing per-transaction and default privacy mode selection, refactoring the send flow to support shielded outputs and compute privacy fees, and tracking private (shielded) versus public (transparent) balance splits. Feature toggles gate shielded address derivation and crypto provider initialization across wallet operations. ChangesiOS Native Crypto Bridge and FFI
Privacy Mode Constants, Models, and Persistence
Privacy Mode UI Components and Screens
Shielded Send Flow: Address Input, Amount Input, and Confirmation
Balance Display and Transaction Fee Tracking
Feature Flag-Based Address Derivation and Navigation Updates
Build Configuration, Metro, Package Updates, and Patches
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
android/app/build.gradle (1)
138-138:⚠️ Potential issue | 🟠 MajorUpdate Firebase BoM to the latest version.
Firebase BoM
31.1.1is significantly outdated. The latest version is34.12.0. Update to the latest version to receive the most recent security patches and features.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/build.gradle` at line 138, Update the Firebase BoM version string in the Gradle dependency declaration: replace the current implementation platform('com.google.firebase:firebase-bom:31.1.1') entry with the latest BoM version implementation platform('com.google.firebase:firebase-bom:34.12.0') so the project uses the newest Firebase releases and security fixes.src/screens/Settings.js (1)
129-132:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Securityitem missingisFirstwhenIS_MULTI_TOKENisfalse.When
IS_MULTI_TOKENisfalseat build time, both token menu entries are filtered out andSecuritybecomes the first visible child of theGENERAL SETTINGSHathorList. WithoutisFirst,BaseItem.firstItemView(borderTopLeftRadius/borderTopRightRadius) is never applied, leaving the card with a squared top edge.🐛 Proposed fix
<ListMenu title={t`Security`} onPress={() => this.props.navigation.navigate('Security')} + isFirst={!IS_MULTI_TOKEN} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Settings.js` around lines 129 - 132, The Security ListMenu lacks the isFirst prop when token entries are removed at build-time, so update the Security item to pass isFirst based on the build flag (e.g. isFirst={!IS_MULTI_TOKEN}) so it becomes the first child when IS_MULTI_TOKEN is false; modify the ListMenu JSX for the 'Security' entry (symbol: ListMenu, title 'Security') to include isFirst and ensure IS_MULTI_TOKEN is referenced/imported in this module so BaseItem.firstItemView (borderTopLeftRadius/borderTopRightRadius) is applied correctly.src/screens/TokenSwapReview.js (1)
125-160:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCall
exitOnError()directly in this catch block.Line 159 still uses
this.exitOnError()inside a function component, so any failure in the send path throws a second error and skips the intended recovery flow.Proposed fix
} catch (err) { console.error(err); - this.exitOnError(); + exitOnError(); } finally { setLoading(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/TokenSwapReview.js` around lines 125 - 160, In the executeSend async function's catch block you're calling this.exitOnError(), which is invalid inside a function component; update the catch to call exitOnError() directly (no this) and ensure the exitOnError identifier is in scope (import or reference the prop/context function used elsewhere); locate executeSend and replace the this.exitOnError() call with exitOnError() and remove any use of this in that handler.
🧹 Nitpick comments (10)
src/components/Icons/Link.icon.js (1)
13-17: 💤 Low valueJSDoc is missing the
typeprop.The component accepts a
typeprop (passed directly toBaseIcon) but the JSDoc block only documentssizeandcolor.📝 Proposed fix
/** * `@param` {object} props + * `@property` {string} props.type * `@property` {number} props.size * `@property` {string} props.color */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Icons/Link.icon.js` around lines 13 - 17, Update the JSDoc for the Link icon component to include the missing `type` prop: add a `@property` entry like "@property {string} props.type" (with a short description, e.g., "icon type passed to BaseIcon") alongside the existing `@property` tags for size and color so the documentation reflects that this component forwards a `type` prop to BaseIcon.src/components/Icons/Shield.icon.js (1)
13-17: 💤 Low valueJSDoc omits the
typeprop.The component signature accepts
typeand passes it toBaseIcon, but the JSDoc block only documentssizeandcolor.📝 Proposed fix
/** * `@param` {object} props + * `@property` {string} props.type * `@property` {number} props.size * `@property` {string} props.color */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Icons/Shield.icon.js` around lines 13 - 17, The JSDoc for the Shield icon component is missing the `type` prop documentation even though the component accepts a `type` prop and passes it to BaseIcon; update the JSDoc block above the Shield component (the comment that documents props.size and props.color) to include `@property {string} props.type` (or the appropriate type/union) and a short description so the signature matches the actual parameters passed to BaseIcon and tools/IDE tooling pick it up correctly.src/screens/SendScanQRCode.js (1)
19-21: 💤 Low valueConsider removing unused Redux state.
The
walletprop frommapStateToPropsis selected but never used in this component. If it's not needed, consider simplifying:-const mapStateToProps = (state) => ({ - wallet: state.wallet, -}); +const mapStateToProps = () => ({});Or if no Redux state is needed, use
connect(null)instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/SendScanQRCode.js` around lines 19 - 21, mapStateToProps is returning wallet but the SendScanQRCode component never uses the wallet prop; either remove wallet from mapStateToProps or remove mapStateToProps entirely and call connect(null)(SendScanQRCode) (or simply export the component without connect) to avoid selecting unused Redux state—update the file by modifying or deleting mapStateToProps and adjusting the connect(...) usage accordingly.src/components/TxDetailsModal.js (1)
119-119: ⚡ Quick winUse dynamic native token symbol from serverInfo instead of hardcoded constant.
Based on learnings, native token symbols should be retrieved dynamically from
serverInfo.native_token.symbolrather than usinghathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol. This ensures the symbol reflects the connected server's configuration.You'll need to either pass
serverInfoas a prop or connect to Redux to access it.♻️ Suggested approach
- const nativeSymbol = hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol; + // Access serverInfo from Redux or props + const nativeSymbol = serverInfo?.native_token?.symbol + ?? hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TxDetailsModal.js` at line 119, Replace the hardcoded native token symbol usage in TxDetailsModal (currently using hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol assigned to nativeSymbol) with the dynamic value from serverInfo.native_token.symbol: update TxDetailsModal to accept serverInfo as a prop (or connect it to Redux to select serverInfo) and read nativeSymbol = serverInfo.native_token.symbol (with a safe fallback to the existing constant if serverInfo or native_token is missing) so the UI reflects the server's configured native token.ios/Podfile (1)
90-98: 💤 Low valueConsolidate duplicate
fmtconsteval patches.There are two separate patches for the
fmt/base.hconsteval issue:
- Lines 90-98: Glob-based patch replacing
FMT_USE_CONSTEVAL 1→FMT_USE_CONSTEVAL 0- Lines 128-146: Specific-path patch with regex replacing
__apple_build_version__ < \d+L→__apple_build_version__These address different root causes but both target the same symptom. Consider consolidating them with a comment explaining why both are needed, or remove one if it's superseded by the other.
Also applies to: 128-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Podfile` around lines 90 - 98, There are duplicate patches for fmt's consteval issues: the Dir.glob loop that replaces 'FMT_USE_CONSTEVAL 1' via text.gsub in the Podfile and the later specific-path regex that adjusts '__apple_build_version__ < \d+L'; consolidate by keeping both fixes in one place with a short comment explaining they target different root causes (the consteval flag vs Apple build-version checks), or remove the redundant one if confirmed unnecessary; update the Podfile to combine the Dir.glob logic and the regex replacement into a single patch block (referencing the Dir.glob loop, the File.read/File.write workflow, the text.gsub for 'FMT_USE_CONSTEVAL', and the regex that touches '__apple_build_version__') and add a comment explaining why both transformations are applied when present.src/shieldedCryptoProvider.js (1)
24-37: TODO: Add dedicatedgenerateRandomBlindingFactorto native module.The workaround using
createShieldedOutputwith dummy values works but is wasteful—it generates a full shielded output just to extract the blinding factor. Consider implementing a dedicated native method as noted in the TODO.Would you like me to open an issue to track adding a dedicated
generateRandomBlindingFactormethod to the native module?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shieldedCryptoProvider.js` around lines 24 - 37, Replace the heavy workaround in generateRandomBlindingFactor by adding and calling a dedicated native method (e.g., HathorCtCryptoModule.generateRandomBlindingFactor) instead of createShieldedOutput; implement the native function to return a 32-byte secp256k1 scalar from the Rust RNG, export it to the JS bridge, update generateRandomBlindingFactor to await that call and return arrToBuf(result) (or equivalent) and remove the TODO; ensure the new native API handles errors and rejects on failure so the JS method can propagate/log them.src/screens/SendConfirmScreen.js (2)
298-310: 💤 Low valueEvent listeners not cleaned up for pre-prepared sendTransaction.
The
send-errorandjob-errorlisteners added at lines 298-305 are never removed. IfexecuteSendis called multiple times (unlikely but possible), listeners accumulate.♻️ Suggested fix
Consider storing listeners and removing them in the modal's dismiss handlers, or use
onceif available:- sendTransaction.on('send-error', (msg) => { + const onSendError = (msg) => { console.log('[SEND] send-error event:', msg); - }); + }; + sendTransaction.once('send-error', onSendError);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/SendConfirmScreen.js` around lines 298 - 310, The added 'send-error' and 'job-error' listeners on sendTransaction are never removed causing accumulation across executeSend calls; update the code around sendTransaction (where sendTransaction.on('send-error', ...) and .on('job-error', ... ) are attached) to either use sendTransaction.once(...) so handlers auto-remove after firing, or capture the handler functions and call sendTransaction.off/removeListener for both 'send-error' and 'job-error' in the modal dismissal/cleanup path (also ensure cleanup runs after runFromMining() completes or on error), leaving signTx and runFromMining usage unchanged.
102-106: ⚡ Quick winError message hardcodes "0.01" balance requirement.
The error message at line 103-105 hardcodes "0.01 balance". This should use the dynamic fee value or at minimum reference the native token symbol dynamically per coding guidelines.
♻️ Suggested fix
} catch (_e) { throw new Error( - 'Insufficient funds: shielded transactions require at least 2 outputs. ' - + 'You need more than 0.01 balance to send a shielded transaction.' + t`Insufficient funds: shielded transactions require at least 2 outputs. ` + + t`You need more balance to send a shielded transaction.` ); }Based on learnings: "Do not hardcode the native token name or symbol in any user-facing UI."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/SendConfirmScreen.js` around lines 102 - 106, The thrown Error in SendConfirmScreen currently hardcodes "0.01" and a token reference; update the throw in the SendConfirmScreen component to use the actual computed minimum/fee value and the native token symbol instead of literals—e.g., replace the hardcoded text with a template string that injects the dynamic minimum or fee variable (such as requiredBalance, minShieldedBalance, or shieldedFee) and the native token symbol (e.g., nativeToken.symbol or getNativeTokenSymbol()), and format the numeric value via the existing balance/formatter utility so the message reads like: `Insufficient funds: shielded transactions require at least 2 outputs. You need more than {formattedRequiredBalance} {tokenSymbol} to send a shielded transaction.` Ensure you import/use the same variables/utilities used elsewhere in SendConfirmScreen for consistency.src/screens/SendAmountInput.js (1)
228-251: ⚡ Quick winReplace hardcoded "HTR" with dynamic native token symbol.
The error messages at lines 232, 237, and 248 hardcode "HTR". Per coding guidelines, native token references should use dynamic values from
serverInfo.native_token.symbol.♻️ Suggested fix
Add the selector:
const nativeSymbol = useSelector( (state) => state.serverInfo?.native_token?.symbol ?? 'HTR' );Then update the error messages:
- setError(t`Insufficient balance of HTR to cover the network fee.`); + setError(t`Insufficient balance of ${nativeSymbol} to cover the network fee.`);Based on learnings: "Do not hardcode the native token name or symbol in any user-facing UI in this repository."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/SendAmountInput.js` around lines 228 - 251, Replace hardcoded "HTR" in user-facing error messages with the dynamic native token symbol: add a selector like const nativeSymbol = useSelector(state => state.serverInfo?.native_token?.symbol ?? 'HTR') in SendAmountInput and then use nativeSymbol in the setError calls (the ones inside the TokenVersion.FEE branch checking htrBalance and networkFee, and in the privacyMode !== 'public' branch that checks htrAvailable vs required) instead of the literal "HTR" so messages reference the configured native token symbol.src/sagas/wallet.js (1)
264-278: ⚡ Quick winUse logger instead of console.log for consistency.
Lines 274 and 276 use
console.logwhile the rest of the file uses theloglogger instance (line 95). For consistent log levels and filtering:♻️ Suggested fix
- console.log('[WALLET] Mobile shielded crypto provider set'); + log.log('[WALLET] Mobile shielded crypto provider set'); } catch (e) { - console.log('Mobile shielded crypto provider not available:', e.message); + log.warn('Mobile shielded crypto provider not available:', e.message); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sagas/wallet.js` around lines 264 - 278, Replace the two console.log calls in the shielded-enabled block with the module logger variable `log` (defined earlier as `log`) to keep logging consistent: use `log.info` for the success message after `wallet.storage.setShieldedCryptoProvider(createMobileShieldedCryptoProvider())`, and use `log.warn` or `log.error` in the catch to include the caught error details (e or e.message) when reporting that the mobile shielded crypto provider is not available; the relevant symbols are `shieldedEnabled`, `checkForFeatureFlag`, `SHIELDED_OUTPUTS_FEATURE_TOGGLE`, `createMobileShieldedCryptoProvider`, `wallet.storage.setShieldedCryptoProvider`, and `log`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/build.gradle`:
- Line 29: The hardcoded Nix path set via nodeExecutableAndArgs in
android/app/build.gradle will break other environments; remove the fixed path or
make it conditional by reading an environment variable (e.g.,
REACT_NATIVE_NODE_EXECUTABLE) and falling back to the default node resolution
when the env var is not set—update the nodeExecutableAndArgs assignment so it
uses System.getenv("REACT_NATIVE_NODE_EXECUTABLE") if present, otherwise leave
the property unset or use the standard resolution.
In `@ios/HathorCtCrypto/HathorCtCryptoModule.swift`:
- Around line 26-27: The code currently uses UInt64(value) which traps on
negative, NaN, or infinite Double inputs and crashes the process; update each
callsite (createShieldedOutputUniffi / createShieldedOutput,
createShieldedOutputWithBlinding, createShieldedOutputWithBothBlindings, and
computeBalancingBlindingFactorFull) to first attempt UInt64(exactly: value) in a
guard/if-let, and if it returns nil reject the Promise with a clear error
instead of forwarding a forced conversion; ensure any remaining uses of
UInt64(value) in these methods are replaced with the safely unwrapped UInt64
from UInt64(exactly:).
- Around line 132-148: The code currently force-casts dictionary values inside
the domain map which can raise Obj-C exceptions and crash; update
createSurjectionProof and computeBalancingBlindingFactorFull to safely unwrap
and validate required keys before mapping: for each domain/inEntry/outEntry use
guard/if-let to extract and type-check "generator", "tag", "blindingFactor" (or
"vbf"/"gbf") as the expected arrays/NSNumbers, reject the promise
(reject("CRYPTO_ERROR", "missing or invalid field: <field>", nil) or similar)
and return early when validation fails, then convert with toData and construct
SurjectionDomainEntry and call createSurjectionProofUniffi only after validation
succeeds so no force-casts remain.
- Around line 9-11: The toData(_ arr: [Any]) -> Data function silently drops
non-NSNumber elements causing truncated byte buffers; change its signature to
throws (toData(_ arr: [Any]) throws -> Data) and validate each element instead
of using compactMap: iterate over arr, attempt to cast each item to NSNumber and
extract .uint8Value, and if any element fails the cast throw a descriptive
NSError/Swift error (e.g., "invalid byte at index X: expected NSNumber"), so
callers in their existing do/catch will surface a clear error instead of
producing truncated crypto buffers.
In `@ios/HathorMobile.xcodeproj/project.pbxproj`:
- Line 489: The MARKETING_VERSION in the Xcode project file is set to a
placeholder value (MARKETING_VERSION = 0.38.99999); update all occurrences of
MARKETING_VERSION to the actual release version string (e.g., 0.38.x) before
merging so the App Store and device Settings show the correct version; locate
and replace every instance of MARKETING_VERSION = 0.38.99999 in the
project.pbxproj (there are multiple entries) with the intended release version.
In `@package.json`:
- Line 3: The package.json "version" value "0.0.1-shielded" regresses the app
version and will break store uploads; update the package.json version field to
use the current production base version with a pre-release suffix (for example
"0.38.0-shielded" or match MARKETING_VERSION like "0.38.99999-shielded") so
CI/release scripts that read package.json don't produce a lower build number;
locate and modify the "version" key in package.json accordingly.
In `@pre_release.sh`:
- Line 16: pre_release.sh currently invokes ./scripts/cleanup.sh without
platform flags, so ios/android-specific cleanup (cleanup_ios.sh,
cleanup_android.sh) is skipped; update the call in pre_release.sh to pass the
platform flags by invoking ./scripts/cleanup.sh --ios --android (or
conditionally include them based on environment) so Pods, DerivedData, Archives
and Gradle caches are removed before pod install, or if skipping is intentional
add a brief comment explaining why the platform cleanup is omitted.
In `@src/actions.js`:
- Around line 612-642: When storage.selectUtxos is missing or iteration fails,
don't return numeric 0n for privateBalance/publicBalance; instead set both
privateBalance and publicBalance to null (or undefined) to signal "unknown" so
the UI can hide the split row. Concretely: in the block around wallet.storage
and storage.selectUtxos (referencing variables privateBalance, publicBalance,
wallet, and the selectUtxos async iteration), initialize them to null and only
assign bigint sums when selectUtxos exists and completes successfully; on any
early exit or caught error leave them null and still return available/locked as
before. Apply the same sentinel change to the mirrored logic in
src/sagas/tokens.js so both fetch paths use null to indicate an unavailable
split.
In `@src/components/PrivacyModeCard.js`:
- Around line 33-63: buildOptions currently hardcodes "HTR" in the feeLabel
strings; change it to read the native token symbol from Redux (e.g.
state.serverInfo.native_token.symbol) with a fallback of 'HTR' if server info
isn't loaded, then interpolate that symbol into the feeLabel for both
FEE_PER_FS_OUTPUT and FEE_PER_AS_OUTPUT entries; update the buildOptions usage
so feeLabel reads `Fee: ${renderValue(FEE_PER_FS_OUTPUT, false, decimalPlaces)}
${nativeSymbol} per output` (and similarly for FEE_PER_AS_OUTPUT) and ensure
PRIVACY_MODE, renderValue and fee constants remain unchanged.
In `@src/components/ReceiveMyAddress.js`:
- Around line 30-52: The UI falls back to lastSharedAddress while an async
shielded derivation is still pending; change the flow so the fallback only
happens after an explicit failure. Initialize displayAddress to undefined and in
useEffect (function derive inside effect that calls wallet.getCurrentAddress)
set displayAddress to the derived address on success and to null on catch; do
not set displayAddress to null before starting derivation. Compute addressToShow
so that when shieldedEnabled you only use displayAddress if it is !== undefined
(treat undefined as "resolving"), otherwise keep the QR/share UI hidden or
disabled when shieldedEnabled && displayAddress === undefined; only fall back to
lastSharedAddress when shieldedEnabled is false or displayAddress === null.
Ensure references: displayAddress, setDisplayAddress, useEffect/derive,
shieldedEnabled, lastSharedAddress, wallet, and addressToShow.
In `@src/constants.js`:
- Line 204: The SHIELDED_OUTPUTS_FEATURE_TOGGLE constant currently defaults to
true while all other feature toggles default to false; update the definition for
SHIELDED_OUTPUTS_FEATURE_TOGGLE to either (a) change its default to false to
match the established pattern or (b) if true is intentional, add/expand the
JSDoc above the SHIELDED_OUTPUTS_FEATURE_TOGGLE constant to explicitly state
that the default true is deliberate (include why: e.g., initial shielded-testnet
rollout, behavior when Unleash is unreachable, and the user-visible
address-format change) so future reviewers understand the exception; reference
the SHIELDED_OUTPUTS_FEATURE_TOGGLE symbol when making this change.
In `@src/screens/SendAmountInput.js`:
- Around line 100-108: Replace direct string comparisons in shieldedFeeForTx and
the other occurrences with the imported PRIVACY_MODE constants: use
PRIVACY_MODE.HIDE_AMOUNT instead of 'hide_amount' and PRIVACY_MODE.PRIVATE
instead of 'private' in the shieldedFeeForTx function and at the other places
noted (lines referenced in the review). Update the conditional checks where
privacyMode === 'hide_amount' and privacyMode === 'private' (and any identical
string literal checks at the other locations) to use PRIVACY_MODE.HIDE_AMOUNT
and PRIVACY_MODE.PRIVATE respectively so the code consistently uses the imported
constants.
---
Outside diff comments:
In `@android/app/build.gradle`:
- Line 138: Update the Firebase BoM version string in the Gradle dependency
declaration: replace the current implementation
platform('com.google.firebase:firebase-bom:31.1.1') entry with the latest BoM
version implementation platform('com.google.firebase:firebase-bom:34.12.0') so
the project uses the newest Firebase releases and security fixes.
In `@src/screens/Settings.js`:
- Around line 129-132: The Security ListMenu lacks the isFirst prop when token
entries are removed at build-time, so update the Security item to pass isFirst
based on the build flag (e.g. isFirst={!IS_MULTI_TOKEN}) so it becomes the first
child when IS_MULTI_TOKEN is false; modify the ListMenu JSX for the 'Security'
entry (symbol: ListMenu, title 'Security') to include isFirst and ensure
IS_MULTI_TOKEN is referenced/imported in this module so BaseItem.firstItemView
(borderTopLeftRadius/borderTopRightRadius) is applied correctly.
In `@src/screens/TokenSwapReview.js`:
- Around line 125-160: In the executeSend async function's catch block you're
calling this.exitOnError(), which is invalid inside a function component; update
the catch to call exitOnError() directly (no this) and ensure the exitOnError
identifier is in scope (import or reference the prop/context function used
elsewhere); locate executeSend and replace the this.exitOnError() call with
exitOnError() and remove any use of this in that handler.
---
Nitpick comments:
In `@ios/Podfile`:
- Around line 90-98: There are duplicate patches for fmt's consteval issues: the
Dir.glob loop that replaces 'FMT_USE_CONSTEVAL 1' via text.gsub in the Podfile
and the later specific-path regex that adjusts '__apple_build_version__ < \d+L';
consolidate by keeping both fixes in one place with a short comment explaining
they target different root causes (the consteval flag vs Apple build-version
checks), or remove the redundant one if confirmed unnecessary; update the
Podfile to combine the Dir.glob logic and the regex replacement into a single
patch block (referencing the Dir.glob loop, the File.read/File.write workflow,
the text.gsub for 'FMT_USE_CONSTEVAL', and the regex that touches
'__apple_build_version__') and add a comment explaining why both transformations
are applied when present.
In `@src/components/Icons/Link.icon.js`:
- Around line 13-17: Update the JSDoc for the Link icon component to include the
missing `type` prop: add a `@property` entry like "@property {string} props.type"
(with a short description, e.g., "icon type passed to BaseIcon") alongside the
existing `@property` tags for size and color so the documentation reflects that
this component forwards a `type` prop to BaseIcon.
In `@src/components/Icons/Shield.icon.js`:
- Around line 13-17: The JSDoc for the Shield icon component is missing the
`type` prop documentation even though the component accepts a `type` prop and
passes it to BaseIcon; update the JSDoc block above the Shield component (the
comment that documents props.size and props.color) to include `@property
{string} props.type` (or the appropriate type/union) and a short description so
the signature matches the actual parameters passed to BaseIcon and tools/IDE
tooling pick it up correctly.
In `@src/components/TxDetailsModal.js`:
- Line 119: Replace the hardcoded native token symbol usage in TxDetailsModal
(currently using hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol assigned
to nativeSymbol) with the dynamic value from serverInfo.native_token.symbol:
update TxDetailsModal to accept serverInfo as a prop (or connect it to Redux to
select serverInfo) and read nativeSymbol = serverInfo.native_token.symbol (with
a safe fallback to the existing constant if serverInfo or native_token is
missing) so the UI reflects the server's configured native token.
In `@src/sagas/wallet.js`:
- Around line 264-278: Replace the two console.log calls in the shielded-enabled
block with the module logger variable `log` (defined earlier as `log`) to keep
logging consistent: use `log.info` for the success message after
`wallet.storage.setShieldedCryptoProvider(createMobileShieldedCryptoProvider())`,
and use `log.warn` or `log.error` in the catch to include the caught error
details (e or e.message) when reporting that the mobile shielded crypto provider
is not available; the relevant symbols are `shieldedEnabled`,
`checkForFeatureFlag`, `SHIELDED_OUTPUTS_FEATURE_TOGGLE`,
`createMobileShieldedCryptoProvider`,
`wallet.storage.setShieldedCryptoProvider`, and `log`.
In `@src/screens/SendAmountInput.js`:
- Around line 228-251: Replace hardcoded "HTR" in user-facing error messages
with the dynamic native token symbol: add a selector like const nativeSymbol =
useSelector(state => state.serverInfo?.native_token?.symbol ?? 'HTR') in
SendAmountInput and then use nativeSymbol in the setError calls (the ones inside
the TokenVersion.FEE branch checking htrBalance and networkFee, and in the
privacyMode !== 'public' branch that checks htrAvailable vs required) instead of
the literal "HTR" so messages reference the configured native token symbol.
In `@src/screens/SendConfirmScreen.js`:
- Around line 298-310: The added 'send-error' and 'job-error' listeners on
sendTransaction are never removed causing accumulation across executeSend calls;
update the code around sendTransaction (where sendTransaction.on('send-error',
...) and .on('job-error', ... ) are attached) to either use
sendTransaction.once(...) so handlers auto-remove after firing, or capture the
handler functions and call sendTransaction.off/removeListener for both
'send-error' and 'job-error' in the modal dismissal/cleanup path (also ensure
cleanup runs after runFromMining() completes or on error), leaving signTx and
runFromMining usage unchanged.
- Around line 102-106: The thrown Error in SendConfirmScreen currently hardcodes
"0.01" and a token reference; update the throw in the SendConfirmScreen
component to use the actual computed minimum/fee value and the native token
symbol instead of literals—e.g., replace the hardcoded text with a template
string that injects the dynamic minimum or fee variable (such as
requiredBalance, minShieldedBalance, or shieldedFee) and the native token symbol
(e.g., nativeToken.symbol or getNativeTokenSymbol()), and format the numeric
value via the existing balance/formatter utility so the message reads like:
`Insufficient funds: shielded transactions require at least 2 outputs. You need
more than {formattedRequiredBalance} {tokenSymbol} to send a shielded
transaction.` Ensure you import/use the same variables/utilities used elsewhere
in SendConfirmScreen for consistency.
In `@src/screens/SendScanQRCode.js`:
- Around line 19-21: mapStateToProps is returning wallet but the SendScanQRCode
component never uses the wallet prop; either remove wallet from mapStateToProps
or remove mapStateToProps entirely and call connect(null)(SendScanQRCode) (or
simply export the component without connect) to avoid selecting unused Redux
state—update the file by modifying or deleting mapStateToProps and adjusting the
connect(...) usage accordingly.
In `@src/shieldedCryptoProvider.js`:
- Around line 24-37: Replace the heavy workaround in
generateRandomBlindingFactor by adding and calling a dedicated native method
(e.g., HathorCtCryptoModule.generateRandomBlindingFactor) instead of
createShieldedOutput; implement the native function to return a 32-byte
secp256k1 scalar from the Rust RNG, export it to the JS bridge, update
generateRandomBlindingFactor to await that call and return arrToBuf(result) (or
equivalent) and remove the TODO; ensure the new native API handles errors and
rejects on failure so the JS method can propagate/log them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83dca92d-6401-43e7-8fe1-6da3ce313301
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (51)
.gitignoreandroid/app/build.gradleios/HathorCtCrypto/HathorCtCryptoModule.mios/HathorCtCrypto/HathorCtCryptoModule.swiftios/HathorCtCrypto/hathor_ct_crypto.swiftios/HathorCtCrypto/hathor_ct_cryptoFFI.hios/HathorCtCrypto/hathor_ct_cryptoFFI.modulemapios/HathorMobile-Bridging-Header.hios/HathorMobile.xcodeproj/project.pbxprojios/Podfilemetro.config.jspackage.jsonpatches/@hathor+wallet-lib+0.0.6-shielded.patchpatches/@hathor+wallet-lib+2.17.0.patchpre_release.shsrc/App.jssrc/actions.jssrc/components/HathorList.jssrc/components/Icons/ChevronDown.icon.jssrc/components/Icons/ChevronUp.icon.jssrc/components/Icons/Eye.icon.jssrc/components/Icons/EyeOff.icon.jssrc/components/Icons/EyeOffSlash.icon.jssrc/components/Icons/Link.icon.jssrc/components/Icons/Shield.icon.jssrc/components/Icons/ShieldPadlock.icon.jssrc/components/PrivacyModeCard.jssrc/components/ReceiveMyAddress.jssrc/components/TransactionPrivacyModal.jssrc/components/TxDetailsModal.jssrc/constants.jssrc/models.jssrc/reducers/reducer.jssrc/sagas/index.jssrc/sagas/privacySettings.jssrc/sagas/reown.jssrc/sagas/tokens.jssrc/sagas/wallet.jssrc/screens/CreateTokenConfirm.jssrc/screens/MainScreen.jssrc/screens/NetworkSettings/NetworkPreSettingsScreen.jssrc/screens/PrivacySettings.jssrc/screens/SendAddressInput.jssrc/screens/SendAmountInput.jssrc/screens/SendConfirmScreen.jssrc/screens/SendScanQRCode.jssrc/screens/Settings.jssrc/screens/TokenSwapReview.jssrc/shieldedCryptoProvider.jssrc/store.jssrc/utils.js
💤 Files with no reviewable changes (1)
- patches/@Hathor+wallet-lib+2.17.0.patch
| /* Bundling */ | ||
| // A list containing the node command and its flags. Default is just 'node'. | ||
| // nodeExecutableAndArgs = ["node"] | ||
| nodeExecutableAndArgs = ["/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node"] |
There was a problem hiding this comment.
Remove or make the hardcoded Nix store path conditional.
The hardcoded Nix store path /nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node will break builds for developers who don't have this exact path (other machines, CI/CD pipelines, non-Nix environments). Nix store paths include derivation-specific hashes that don't transfer between systems.
Either remove this configuration to use the default Node resolution, or make it conditional using an environment variable:
🔧 Proposed fix using environment variable
- nodeExecutableAndArgs = ["/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node"]
+ // Use custom Node path from environment if available (e.g., for Nix devshell)
+ def customNode = System.getenv("REACT_NATIVE_NODE_EXECUTABLE")
+ if (customNode) {
+ nodeExecutableAndArgs = [customNode]
+ }Then developers using Nix can set REACT_NATIVE_NODE_EXECUTABLE in their environment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| nodeExecutableAndArgs = ["/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node"] | |
| // Use custom Node path from environment if available (e.g., for Nix devshell) | |
| def customNode = System.getenv("REACT_NATIVE_NODE_EXECUTABLE") | |
| if (customNode) { | |
| nodeExecutableAndArgs = [customNode] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@android/app/build.gradle` at line 29, The hardcoded Nix path set via
nodeExecutableAndArgs in android/app/build.gradle will break other environments;
remove the fixed path or make it conditional by reading an environment variable
(e.g., REACT_NATIVE_NODE_EXECUTABLE) and falling back to the default node
resolution when the env var is not set—update the nodeExecutableAndArgs
assignment so it uses System.getenv("REACT_NATIVE_NODE_EXECUTABLE") if present,
otherwise leave the property unset or use the standard resolution.
| private func toData(_ arr: [Any]) -> Data { | ||
| Data(arr.compactMap { ($0 as? NSNumber)?.uint8Value }) | ||
| } |
There was a problem hiding this comment.
toData silently drops non-NSNumber bytes — cryptographic truncation with no error
compactMap { ($0 as? NSNumber)?.uint8Value } discards any element that isn't an NSNumber. For a 32-byte key where one element is unexpectedly not an NSNumber, the crypto call proceeds with a 31-byte buffer. The UniFFI/Rust layer will then produce a cryptographic error whose message won't indicate the real cause (wrong input length).
🛡️ Proposed fix — reject on type mismatch
- private func toData(_ arr: [Any]) -> Data {
- Data(arr.compactMap { ($0 as? NSNumber)?.uint8Value })
- }
+ private func toData(_ arr: [Any]) throws -> Data {
+ let bytes: [UInt8] = try arr.map {
+ guard let n = $0 as? NSNumber else {
+ throw NSError(domain: "HathorCtCrypto", code: -1,
+ userInfo: [NSLocalizedDescriptionKey: "Expected NSNumber in byte array, got \(type(of: $0))"])
+ }
+ return n.uint8Value
+ }
+ return Data(bytes)
+ }All call sites already sit inside do/catch blocks that forward errors to reject(...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/HathorCtCrypto/HathorCtCryptoModule.swift` around lines 9 - 11, The
toData(_ arr: [Any]) -> Data function silently drops non-NSNumber elements
causing truncated byte buffers; change its signature to throws (toData(_ arr:
[Any]) throws -> Data) and validate each element instead of using compactMap:
iterate over arr, attempt to cast each item to NSNumber and extract .uint8Value,
and if any element fails the cast throw a descriptive NSError/Swift error (e.g.,
"invalid byte at index X: expected NSNumber"), so callers in their existing
do/catch will surface a clear error instead of producing truncated crypto
buffers.
| let domainEntries = domain.map { entry -> SurjectionDomainEntry in | ||
| SurjectionDomainEntry( | ||
| generator: self.toData(entry["generator"] as! [Any]), | ||
| tag: self.toData(entry["tag"] as! [Any]), | ||
| blindingFactor: self.toData(entry["blindingFactor"] as! [Any]) | ||
| ) | ||
| } | ||
| let result = try createSurjectionProofUniffi( | ||
| codomainTag: toData(codomainTag), | ||
| codomainBlindingFactor: toData(codomainBlindingFactor), | ||
| domain: domainEntries | ||
| ) | ||
| resolve(toArray(result)) | ||
| } catch { | ||
| reject("CRYPTO_ERROR", error.localizedDescription, error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Force-cast dictionary lookups crash the app instead of rejecting the promise
The as! [Any] and as! NSNumber force-casts in createSurjectionProof (Lines 134–136) and computeBalancingBlindingFactorFull (Lines 235–244) execute inside a map closure that runs synchronously in the do block. If a domain entry is missing a required key (e.g., "generator", "vbf", "gbf") or carries an unexpected type, the force-cast throws an Objective-C exception that bypasses Swift's do/catch and crashes the process.
🐛 Proposed fix — safe unwrapping with rejection
- do {
- let domainEntries = domain.map { entry -> SurjectionDomainEntry in
- SurjectionDomainEntry(
- generator: self.toData(entry["generator"] as! [Any]),
- tag: self.toData(entry["tag"] as! [Any]),
- blindingFactor: self.toData(entry["blindingFactor"] as! [Any])
- )
- }
+ do {
+ guard domain.allSatisfy({ $0["generator"] is [Any] && $0["tag"] is [Any] && $0["blindingFactor"] is [Any] }) else {
+ reject("INVALID_INPUT", "domain entries must contain generator, tag, and blindingFactor arrays", nil)
+ return
+ }
+ let domainEntries = domain.map { entry -> SurjectionDomainEntry in
+ SurjectionDomainEntry(
+ generator: self.toData(entry["generator"] as! [Any]),
+ tag: self.toData(entry["tag"] as! [Any]),
+ blindingFactor: self.toData(entry["blindingFactor"] as! [Any])
+ )
+ }Apply the same pattern (pre-validate then cast) for the inEntries/outEntries in computeBalancingBlindingFactorFull.
Also applies to: 233-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/HathorCtCrypto/HathorCtCryptoModule.swift` around lines 132 - 148, The
code currently force-casts dictionary values inside the domain map which can
raise Obj-C exceptions and crash; update createSurjectionProof and
computeBalancingBlindingFactorFull to safely unwrap and validate required keys
before mapping: for each domain/inEntry/outEntry use guard/if-let to extract and
type-check "generator", "tag", "blindingFactor" (or "vbf"/"gbf") as the expected
arrays/NSNumbers, reject the promise (reject("CRYPTO_ERROR", "missing or invalid
field: <field>", nil) or similar) and return early when validation fails, then
convert with toData and construct SurjectionDomainEntry and call
createSurjectionProofUniffi only after validation succeeds so no force-casts
remain.
| "$(inherited)", | ||
| "$(PROJECT_DIR)/HathorCtCrypto", | ||
| ); | ||
| MARKETING_VERSION = 0.38.99999; |
There was a problem hiding this comment.
Placeholder marketing version detected.
MARKETING_VERSION = 0.38.99999 appears to be a placeholder value. Ensure this is updated to the actual release version before merging to master, as this value appears in the App Store and on the device's Settings → About screen.
Also applies to: 524-524
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/HathorMobile.xcodeproj/project.pbxproj` at line 489, The
MARKETING_VERSION in the Xcode project file is set to a placeholder value
(MARKETING_VERSION = 0.38.99999); update all occurrences of MARKETING_VERSION to
the actual release version string (e.g., 0.38.x) before merging so the App Store
and device Settings show the correct version; locate and replace every instance
of MARKETING_VERSION = 0.38.99999 in the project.pbxproj (there are multiple
entries) with the intended release version.
| let privateBalance = 0n; | ||
| let publicBalance = 0n; | ||
| try { | ||
| const { storage } = wallet; | ||
| if (storage && storage.selectUtxos) { | ||
| const baseFilter = { token: uid, only_available_utxos: true }; | ||
| for await (const utxo of storage.selectUtxos({ ...baseFilter, shielded: true })) { | ||
| if ((utxo.authorities ?? 0n) !== 0n) continue; | ||
| if (!utxo.value) continue; | ||
| privateBalance += utxo.value; | ||
| } | ||
| for await (const utxo of storage.selectUtxos({ ...baseFilter, shielded: false })) { | ||
| if ((utxo.authorities ?? 0n) !== 0n) continue; | ||
| if (!utxo.value) continue; | ||
| publicBalance += utxo.value; | ||
| } | ||
| } | ||
| } catch (e) { | ||
| // Surface storage iteration failures via the logger so they're | ||
| // diagnosable, but don't block balance fetch — the total still | ||
| // renders from the wallet-lib's cached balance. | ||
| // eslint-disable-next-line no-console | ||
| console.warn('fetchTokenBalance private/public split failed', uid, e?.message ?? e); | ||
| } | ||
|
|
||
| return { | ||
| available: tokenBalance.unlocked, | ||
| locked: tokenBalance.locked, | ||
| privateBalance, | ||
| publicBalance, | ||
| }; |
There was a problem hiding this comment.
Don't encode “split unavailable” as a real 0n / 0n balance.
This fallback makes the new breakdown lie whenever selectUtxos is missing or fails: available can stay positive while privateBalance and publicBalance render as real zeros. The UI needs an explicit “unknown/unavailable” state here instead of a numeric one.
I'd return null/undefined for privateBalance and publicBalance when the split cannot be computed, then hide the split row in the UI until real data exists. The mirrored helper in src/sagas/tokens.js needs the same sentinel so both fetch paths stay consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions.js` around lines 612 - 642, When storage.selectUtxos is missing
or iteration fails, don't return numeric 0n for privateBalance/publicBalance;
instead set both privateBalance and publicBalance to null (or undefined) to
signal "unknown" so the UI can hide the split row. Concretely: in the block
around wallet.storage and storage.selectUtxos (referencing variables
privateBalance, publicBalance, wallet, and the selectUtxos async iteration),
initialize them to null and only assign bigint sums when selectUtxos exists and
completes successfully; on any early exit or caught error leave them null and
still return available/locked as before. Apply the same sentinel change to the
mirrored logic in src/sagas/tokens.js so both fetch paths use null to indicate
an unavailable split.
| const buildOptions = (variant, decimalPlaces) => { | ||
| const isDefault = variant === 'default'; | ||
| return [ | ||
| { | ||
| key: PRIVACY_MODE.PRIVATE, | ||
| label: t`Private`, | ||
| description: isDefault | ||
| ? t`Token and amount hidden on all new transactions` | ||
| : t`Token and amount hidden`, | ||
| feeLabel: t`Fee: ${renderValue(FEE_PER_FS_OUTPUT, false, decimalPlaces)} HTR per output`, | ||
| Icon: ShieldPadlockIcon, | ||
| }, | ||
| { | ||
| key: PRIVACY_MODE.HIDE_AMOUNT, | ||
| label: t`Hide amount`, | ||
| description: isDefault | ||
| ? t`Token visible, amount hidden on all new transactions` | ||
| : t`Token visible, amount hidden`, | ||
| feeLabel: t`Fee: ${renderValue(FEE_PER_AS_OUTPUT, false, decimalPlaces)} HTR per output`, | ||
| Icon: EyeOffSlashIcon, | ||
| }, | ||
| { | ||
| key: PRIVACY_MODE.PUBLIC, | ||
| label: t`Public`, | ||
| description: isDefault | ||
| ? t`Token and amount visible on all new transactions` | ||
| : t`Token and amount visible`, | ||
| feeLabel: t`No privacy fees`, | ||
| Icon: EyeIcon, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Use the server-provided native symbol in these fee labels.
Both fee pills hardcode HTR, so the UI will show the wrong native symbol on networks where serverInfo.native_token.symbol differs. Pull the symbol from Redux here and keep HTR only as a temporary fallback if server info is not loaded yet.
Based on learnings, do not hardcode the native token name or symbol in any user-facing UI in this repository. Retrieve token metadata dynamically from the connected fullnode/wallet service instead of hardcoding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/PrivacyModeCard.js` around lines 33 - 63, buildOptions
currently hardcodes "HTR" in the feeLabel strings; change it to read the native
token symbol from Redux (e.g. state.serverInfo.native_token.symbol) with a
fallback of 'HTR' if server info isn't loaded, then interpolate that symbol into
the feeLabel for both FEE_PER_FS_OUTPUT and FEE_PER_AS_OUTPUT entries; update
the buildOptions usage so feeLabel reads `Fee: ${renderValue(FEE_PER_FS_OUTPUT,
false, decimalPlaces)} ${nativeSymbol} per output` (and similarly for
FEE_PER_AS_OUTPUT) and ensure PRIVACY_MODE, renderValue and fee constants remain
unchanged.
| const [displayAddress, setDisplayAddress] = useState(null); | ||
|
|
||
| const getNextAddress = async () => { | ||
| const { address, index } = await wallet.getNextAddress(); | ||
| // When shielded is enabled, derive the shielded address for the current index. | ||
| // When disabled, fall back to the legacy lastSharedAddress without deriving. | ||
| useEffect(() => { | ||
| if (!lastSharedAddress || !wallet || !shieldedEnabled) { | ||
| setDisplayAddress(null); | ||
| return undefined; | ||
| } | ||
| let cancelled = false; | ||
| const derive = async () => { | ||
| try { | ||
| const info = await wallet.getCurrentAddress({}, { legacy: false }); | ||
| if (!cancelled) setDisplayAddress(info.address); | ||
| } catch (e) { | ||
| if (!cancelled) setDisplayAddress(null); | ||
| } | ||
| }; | ||
| derive(); | ||
| return () => { cancelled = true; }; | ||
| }, [shieldedEnabled, lastSharedAddress, wallet]); | ||
|
|
||
| const addressToShow = displayAddress || lastSharedAddress; |
There was a problem hiding this comment.
Don't fall back to lastSharedAddress while the shielded address is still resolving.
With the current displayAddress || lastSharedAddress flow, shielded mode briefly renders and shares the legacy address until getCurrentAddress(..., { legacy: false }) resolves. That creates a reachable wrong-address window for QR/copy/share.
One way to remove the wrong-address window
- const [displayAddress, setDisplayAddress] = useState(null);
+ // undefined = still resolving, null = resolution failed
+ const [displayAddress, setDisplayAddress] = useState(undefined);
...
- const addressToShow = displayAddress || lastSharedAddress;
+ const addressToShow = shieldedEnabled ? displayAddress : lastSharedAddress;Then keep the QR/share UI hidden or disabled while shieldedEnabled && displayAddress === undefined, and only fall back after an explicit resolution failure.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [displayAddress, setDisplayAddress] = useState(null); | |
| const getNextAddress = async () => { | |
| const { address, index } = await wallet.getNextAddress(); | |
| // When shielded is enabled, derive the shielded address for the current index. | |
| // When disabled, fall back to the legacy lastSharedAddress without deriving. | |
| useEffect(() => { | |
| if (!lastSharedAddress || !wallet || !shieldedEnabled) { | |
| setDisplayAddress(null); | |
| return undefined; | |
| } | |
| let cancelled = false; | |
| const derive = async () => { | |
| try { | |
| const info = await wallet.getCurrentAddress({}, { legacy: false }); | |
| if (!cancelled) setDisplayAddress(info.address); | |
| } catch (e) { | |
| if (!cancelled) setDisplayAddress(null); | |
| } | |
| }; | |
| derive(); | |
| return () => { cancelled = true; }; | |
| }, [shieldedEnabled, lastSharedAddress, wallet]); | |
| const addressToShow = displayAddress || lastSharedAddress; | |
| // undefined = still resolving, null = resolution failed | |
| const [displayAddress, setDisplayAddress] = useState(undefined); | |
| // When shielded is enabled, derive the shielded address for the current index. | |
| // When disabled, fall back to the legacy lastSharedAddress without deriving. | |
| useEffect(() => { | |
| if (!lastSharedAddress || !wallet || !shieldedEnabled) { | |
| setDisplayAddress(null); | |
| return undefined; | |
| } | |
| let cancelled = false; | |
| const derive = async () => { | |
| try { | |
| const info = await wallet.getCurrentAddress({}, { legacy: false }); | |
| if (!cancelled) setDisplayAddress(info.address); | |
| } catch (e) { | |
| if (!cancelled) setDisplayAddress(null); | |
| } | |
| }; | |
| derive(); | |
| return () => { cancelled = true; }; | |
| }, [shieldedEnabled, lastSharedAddress, wallet]); | |
| const addressToShow = shieldedEnabled ? displayAddress : lastSharedAddress; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ReceiveMyAddress.js` around lines 30 - 52, The UI falls back
to lastSharedAddress while an async shielded derivation is still pending; change
the flow so the fallback only happens after an explicit failure. Initialize
displayAddress to undefined and in useEffect (function derive inside effect that
calls wallet.getCurrentAddress) set displayAddress to the derived address on
success and to null on catch; do not set displayAddress to null before starting
derivation. Compute addressToShow so that when shieldedEnabled you only use
displayAddress if it is !== undefined (treat undefined as "resolving"),
otherwise keep the QR/share UI hidden or disabled when shieldedEnabled &&
displayAddress === undefined; only fall back to lastSharedAddress when
shieldedEnabled is false or displayAddress === null. Ensure references:
displayAddress, setDisplayAddress, useEffect/derive, shieldedEnabled,
lastSharedAddress, wallet, and addressToShow.
| [SAFE_BIOMETRY_MODE_FEATURE_TOGGLE]: false, | ||
| [TOKEN_SWAP_FEATURE_TOGGLE]: false, | ||
| [FBT_FEATURE_TOGGLE]: false, | ||
| [SHIELDED_OUTPUTS_FEATURE_TOGGLE]: true, |
There was a problem hiding this comment.
SHIELDED_OUTPUTS_FEATURE_TOGGLE is the only feature toggle that defaults to true.
Every other toggle (WALLET_SERVICE, PUSH_NOTIFICATION, REOWN, NETWORK_SETTINGS, NANO_CONTRACT, SAFE_BIOMETRY_MODE, TOKEN_SWAP, FBT) defaults to false. Shielded outputs is the only one that defaults to true.
This means if Unleash is unreachable at startup, shielded address derivation (which changes the address format presented to users) activates immediately for all wallets. If this is intentional for the initial shielded-testnet rollout, it's worth documenting explicitly in the constant's JSDoc comment so future reviewers understand the deliberate departure from the established pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/constants.js` at line 204, The SHIELDED_OUTPUTS_FEATURE_TOGGLE constant
currently defaults to true while all other feature toggles default to false;
update the definition for SHIELDED_OUTPUTS_FEATURE_TOGGLE to either (a) change
its default to false to match the established pattern or (b) if true is
intentional, add/expand the JSDoc above the SHIELDED_OUTPUTS_FEATURE_TOGGLE
constant to explicitly state that the default true is deliberate (include why:
e.g., initial shielded-testnet rollout, behavior when Unleash is unreachable,
and the user-visible address-format change) so future reviewers understand the
exception; reference the SHIELDED_OUTPUTS_FEATURE_TOGGLE symbol when making this
change.
| const shieldedFeeForTx = () => { | ||
| if (privacyMode === 'hide_amount') { | ||
| return hathorLib.constants.FEE_PER_AMOUNT_SHIELDED_OUTPUT * 2n; | ||
| } | ||
| if (privacyMode === 'private') { | ||
| return hathorLib.constants.FEE_PER_FULL_SHIELDED_OUTPUT * 2n; | ||
| } | ||
| return 0n; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use PRIVACY_MODE constants instead of string literals.
The shieldedFeeForTx function uses string literals 'hide_amount' and 'private' for comparison. The PRIVACY_MODE constants are already imported (line 27-28) and should be used consistently to prevent typo-related bugs.
♻️ Suggested fix
const shieldedFeeForTx = () => {
- if (privacyMode === 'hide_amount') {
+ if (privacyMode === PRIVACY_MODE.HIDE_AMOUNT) {
return hathorLib.constants.FEE_PER_AMOUNT_SHIELDED_OUTPUT * 2n;
}
- if (privacyMode === 'private') {
+ if (privacyMode === PRIVACY_MODE.PRIVATE) {
return hathorLib.constants.FEE_PER_FULL_SHIELDED_OUTPUT * 2n;
}
return 0n;
};Also apply the same fix at lines 145, 149, 202, and 240.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const shieldedFeeForTx = () => { | |
| if (privacyMode === 'hide_amount') { | |
| return hathorLib.constants.FEE_PER_AMOUNT_SHIELDED_OUTPUT * 2n; | |
| } | |
| if (privacyMode === 'private') { | |
| return hathorLib.constants.FEE_PER_FULL_SHIELDED_OUTPUT * 2n; | |
| } | |
| return 0n; | |
| }; | |
| const shieldedFeeForTx = () => { | |
| if (privacyMode === PRIVACY_MODE.HIDE_AMOUNT) { | |
| return hathorLib.constants.FEE_PER_AMOUNT_SHIELDED_OUTPUT * 2n; | |
| } | |
| if (privacyMode === PRIVACY_MODE.PRIVATE) { | |
| return hathorLib.constants.FEE_PER_FULL_SHIELDED_OUTPUT * 2n; | |
| } | |
| return 0n; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/SendAmountInput.js` around lines 100 - 108, Replace direct string
comparisons in shieldedFeeForTx and the other occurrences with the imported
PRIVACY_MODE constants: use PRIVACY_MODE.HIDE_AMOUNT instead of 'hide_amount'
and PRIVACY_MODE.PRIVATE instead of 'private' in the shieldedFeeForTx function
and at the other places noted (lines referenced in the review). Update the
conditional checks where privacyMode === 'hide_amount' and privacyMode ===
'private' (and any identical string literal checks at the other locations) to
use PRIVACY_MODE.HIDE_AMOUNT and PRIVACY_MODE.PRIVATE respectively so the code
consistently uses the imported constants.
End-to-end mobile experience for the shielded-outputs feature in
@hathor/wallet-lib 0.0.7-shielded. The send flow now matches the new
figma, summary fees match the on-chain fee byte-for-byte, and the
tx-detail modal exposes the per-tx fee breakdown.
Send flow (SendAddressInput, SendAmountInput, SendConfirmScreen, SendScanQRCode)
- Manual address entry is the first screen of the Send tab; back button
removed since this is now the entry point.
- QR scanner reachable from a top-right icon, returns to the address
screen with the address pre-filled (in-stack navigation, not push).
- Tap-outside-to-dismiss the keyboard on the address screen; tap
anywhere on the input card to focus the multiline TextInput.
- Address screen wraps long shielded addresses (~95 chars) in a
multi-line input.
- Privacy mode (Public / Hide amount / Private) selectable per-tx via
a bottom-sheet modal; persisted per-wallet default in PrivacySettings.
- SendConfirmScreen now PRE-PREPARES the transaction at mount time
(`prepareTx()` for full-node shielded path) so the summary shows the
actual fee — including the auto-added shielded HTR fee-change that
bumps the shielded-output count from 2 to 3. On confirm, `signTx(pin)`
+ `runFromMining()` reuse the prepared Transaction; the on-chain
bytes are byte-for-byte the previewed ones.
- Fee row in the summary uses the typed `getFeeHeader()` accessor and
classifies `Insufficient amount of tokens` as a user-friendly error.
- HTR fee-change inherits the same privacy mode as the rest of the tx
(via the new `changeShieldedMode` option on
`wallet.sendManyOutputsSendTransaction`).
Token detail (MainScreen)
- "Available Balance" shows the unlocked total (shielded + transparent).
- Inline split below the headline: shield-padlock icon + private value
(sum of unlocked shielded UTXOs), eye icon + public value (sum of
unlocked transparent UTXOs). When values overflow horizontally they
stack vertically (flexWrap).
- Saga-side `fetchTokenBalance` computes the private/public split via
`storage.selectUtxos({ shielded: true|false, only_available_utxos: true })`
so the values match `private + public === unlocked`.
Tx detail modal (TxDetailsModal)
- New "Fees" row with expandable breakdown matching the figma. Total
fee comes from the FeeHeader entries (when persisted on the tx);
privacy fee from per-shielded-output mode counts; network fee is the
residual. Fees precomputed during `TxHistory.from()` so the modal
works against full-node and wallet-service history shapes.
- Row spacing, label/value typography, and dividers brought in line
with the figma (matches the Date / Transaction ID / Public Explorer
rows). When expanded the divider collapses so the row reads as one
cluster with its breakdown.
Settings (Settings, HathorList)
- "About" merged into the bottom info card alongside "Unique app
identifier" and "Connected to" — same shape as the figma.
- HathorList auto-marks the last visible child as `isLast` (filtering
out falsy children from `flag && <ListMenu/>` patterns) so the
trailing rounded corners and removed bottom border come for free
without callers tracking which item is last.
Privacy Settings (new screen)
- White background (figma).
- Three radio cards (Public / Hide amount / Private) with per-card
fee disclosure, persisted via the new `privacySettings` saga.
Native version
- iOS `CURRENT_PROJECT_VERSION` bumped to 0.5.0.
- @hathor/wallet-lib pinned to 0.0.7-shielded; bigint patch updated to
the new version filename and re-applied (corejs-pure JSON.parse for
RN's missing `context` arg).
Sparse-decode receive-side balance fix (wallet-lib 0.0.7-shielded)
- The wallet-lib release bundles the receive-side fix for the
sparse-decode shielded input enrichment that caused balance
doubling on every shielded round-trip; mobile picks it up
automatically via the version bump.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Track the iOS-side ct-crypto bridge so the project builds without
external setup beyond `pod install`:
- HathorCtCryptoModule.{m,swift} — hand-written RCT bridge wiring the
JS-side `shieldedCryptoProvider` calls into the UniFFI Swift API.
- hathor_ct_crypto.swift, hathor_ct_cryptoFFI.h,
hathor_ct_cryptoFFI.modulemap — UniFFI-generated bindings, committed
so contributors don't need a Rust toolchain just to compile the iOS
app. Regenerate via the upstream crate when the FFI surface changes.
Exclude libhathor_ct_crypto.xcframework/ from git — it's a 38 MB
compiled artifact built separately (matches the existing
@hathor/ct-crypto-node prebuild model where CI / a build script
provides the binary).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pedroferreira1
force-pushed
the
feat/shielded-outputs
branch
from
0f08c7f to
134491b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/screens/SendScanQRCode.js (1)
19-21: ⚡ Quick winRemove the unused
walletsubscription frommapStateToProps.
walletis mapped from Redux state but is never referenced inshowAlertError,onSuccess, orrender. The live subscription causes the component to re-render on every wallet state change for no benefit.♻️ Proposed fix
-const mapStateToProps = (state) => ({ - wallet: state.wallet, -}); - class SendScanQRCode extends React.Component {And at the bottom:
-export default connect(mapStateToProps)(SendScanQRCode); +export default SendScanQRCode;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/SendScanQRCode.js` around lines 19 - 21, mapStateToProps currently subscribes to wallet but the component (methods showAlertError, onSuccess, render) never uses it, causing unnecessary re-renders; remove the wallet mapping by changing mapStateToProps to return an empty object (or null) and update the connect call to use that new mapStateToProps (or pass null if you prefer) so the component no longer subscribes to wallet updates and only receives needed props.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TxDetailsModal.js`:
- Line 119: TxDetailsModal currently reads the native token symbol from
hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol (nativeSymbol) which
ignores the connected network; change it to use the network-provided symbol at
serverInfo.native_token.symbol by either passing serverInfo from MainScreen into
TxDetailsModal (add serverInfo to the component props and use
props.serverInfo.native_token.symbol where nativeSymbol is used) or convert
TxDetailsModal into a functional component and call useSelector(state =>
state.server.serverInfo.native_token.symbol) to derive nativeSymbol; update all
references to the old DEFAULT_NATIVE_TOKEN_CONFIG.symbol to use the new value
(retain fallback to DEFAULT_NATIVE_TOKEN_CONFIG.symbol if
serverInfo.native_token.symbol is undefined).
In `@src/screens/SendAddressInput.js`:
- Line 52: The current selector creates a new Network instance on every call
which breaks reference-equality and triggers pointless re-renders; instead, have
useSelector return the plain network value (e.g., state.networkSettings.network)
and then create/memoize the Network instance inside the component using useMemo
so it only changes when that primitive changes—replace the selector that uses
new Network(...) with a selector that returns the raw value and move the Network
construction to a useMemo([networkValue]) block (references: useSelector,
useMemo, Network, networkSettings.network in SendAddressInput.js).
- Line 144: Replace the deprecated prop autoCompleteType with the supported
autoComplete on the TextInput in SendAddressInput.js: find the TextInput (or the
component rendering address input where autoCompleteType='off' is set) and
change it to autoComplete='off' so React Native 0.77 will honor the autocomplete
suppression; keep the same placement and any other props/handlers (e.g., value,
onChangeText) intact.
In `@src/shieldedCryptoProvider.js`:
- Around line 24-37: generateRandomBlindingFactor currently fakes a blinding
factor by calling HathorCtCryptoModule.createShieldedOutput with dummyPubkey and
dummyToken which is fragile; add a proper native API (e.g.,
HathorCtCryptoModule.generateRandomBlindingFactor) that returns a secure
secp256k1 scalar, update the JS generateRandomBlindingFactor to call this new
native method (remove dummyPubkey/dummyToken usage), and ensure the JS wrapper
converts the returned array to a Buffer via arrToBuf like the current code does;
also update native bindings and tests to expose and validate the new
generateRandomBlindingFactor method.
---
Nitpick comments:
In `@src/screens/SendScanQRCode.js`:
- Around line 19-21: mapStateToProps currently subscribes to wallet but the
component (methods showAlertError, onSuccess, render) never uses it, causing
unnecessary re-renders; remove the wallet mapping by changing mapStateToProps to
return an empty object (or null) and update the connect call to use that new
mapStateToProps (or pass null if you prefer) so the component no longer
subscribes to wallet updates and only receives needed props.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e872e1e-e44e-438c-aae1-4087070e17dc
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (50)
.gitignoreios/HathorCtCrypto/HathorCtCryptoModule.mios/HathorCtCrypto/HathorCtCryptoModule.swiftios/HathorCtCrypto/hathor_ct_crypto.swiftios/HathorCtCrypto/hathor_ct_cryptoFFI.hios/HathorCtCrypto/hathor_ct_cryptoFFI.modulemapios/HathorMobile-Bridging-Header.hios/HathorMobile.xcodeproj/project.pbxprojios/Podfilemetro.config.jspackage.jsonpatches/@hathor+wallet-lib+0.0.6-shielded.patchpatches/@hathor+wallet-lib+2.17.0.patchpre_release.shsrc/App.jssrc/actions.jssrc/components/HathorList.jssrc/components/Icons/ChevronDown.icon.jssrc/components/Icons/ChevronUp.icon.jssrc/components/Icons/Eye.icon.jssrc/components/Icons/EyeOff.icon.jssrc/components/Icons/EyeOffSlash.icon.jssrc/components/Icons/Link.icon.jssrc/components/Icons/Shield.icon.jssrc/components/Icons/ShieldPadlock.icon.jssrc/components/PrivacyModeCard.jssrc/components/ReceiveMyAddress.jssrc/components/TransactionPrivacyModal.jssrc/components/TxDetailsModal.jssrc/constants.jssrc/models.jssrc/reducers/reducer.jssrc/sagas/index.jssrc/sagas/privacySettings.jssrc/sagas/reown.jssrc/sagas/tokens.jssrc/sagas/wallet.jssrc/screens/CreateTokenConfirm.jssrc/screens/MainScreen.jssrc/screens/NetworkSettings/NetworkPreSettingsScreen.jssrc/screens/PrivacySettings.jssrc/screens/SendAddressInput.jssrc/screens/SendAmountInput.jssrc/screens/SendConfirmScreen.jssrc/screens/SendScanQRCode.jssrc/screens/Settings.jssrc/screens/TokenSwapReview.jssrc/shieldedCryptoProvider.jssrc/store.jssrc/utils.js
💤 Files with no reviewable changes (1)
- patches/@Hathor+wallet-lib+2.17.0.patch
✅ Files skipped from review due to trivial changes (15)
- ios/HathorCtCrypto/hathor_ct_cryptoFFI.modulemap
- ios/HathorMobile-Bridging-Header.h
- src/components/Icons/Shield.icon.js
- src/components/Icons/Eye.icon.js
- src/components/Icons/EyeOffSlash.icon.js
- src/components/Icons/ChevronUp.icon.js
- src/screens/PrivacySettings.js
- src/components/Icons/Link.icon.js
- src/screens/NetworkSettings/NetworkPreSettingsScreen.js
- ios/HathorCtCrypto/HathorCtCryptoModule.m
- src/store.js
- .gitignore
- src/constants.js
- ios/HathorCtCrypto/hathor_ct_cryptoFFI.h
- ios/HathorCtCrypto/hathor_ct_crypto.swift
🚧 Files skipped from review as they are similar to previous changes (27)
- src/sagas/index.js
- src/components/Icons/EyeOff.icon.js
- src/components/Icons/ShieldPadlock.icon.js
- src/sagas/tokens.js
- src/screens/CreateTokenConfirm.js
- src/components/HathorList.js
- src/sagas/privacySettings.js
- src/screens/TokenSwapReview.js
- src/components/PrivacyModeCard.js
- src/utils.js
- src/sagas/reown.js
- metro.config.js
- pre_release.sh
- src/components/Icons/ChevronDown.icon.js
- package.json
- src/models.js
- src/reducers/reducer.js
- src/components/ReceiveMyAddress.js
- src/App.js
- ios/HathorMobile.xcodeproj/project.pbxproj
- src/sagas/wallet.js
- ios/HathorCtCrypto/HathorCtCryptoModule.swift
- src/actions.js
- ios/Podfile
- src/screens/SendAmountInput.js
- src/screens/Settings.js
- src/screens/SendConfirmScreen.js
| renderFeesRow(networkFee, privacyFee) { | ||
| const totalFee = networkFee + privacyFee; | ||
| if (totalFee <= 0n) return null; | ||
| const nativeSymbol = hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current symbol-source patterns and where dynamic network settings are available.
rg -n --type=js -C3 'DEFAULT_NATIVE_TOKEN_CONFIG\.symbol|native_token\.symbol|getNetworkSettings|serverInfo'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 26170
🏁 Script executed:
# Check TxDetailsModal structure and if it has access to serverInfo
head -150 src/components/TxDetailsModal.js | tail -50Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1787
🏁 Script executed:
# Search for how serverInfo.native_token or similar is used/structured
rg -n "native_token|nativeSymbol|NATIVE_TOKEN" --type=js -C2 | head -100Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 6876
🏁 Script executed:
# Check if TxDetailsModal is a class or functional component and where it gets props/data
ast-grep --pattern $'const TxDetailsModal = ($_) => {
$$$
}'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Look at reducer to see exact serverInfo structure
rg -A 20 "serverInfo: {" src/reducers/reducer.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 168
🏁 Script executed:
# Get the full component structure and props of TxDetailsModal
head -50 src/components/TxDetailsModal.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1631
🏁 Script executed:
# Check how TxDetailsModal is exported and connected to Redux
tail -50 src/components/TxDetailsModal.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1213
🏁 Script executed:
# Check if TxDetailsModal already uses Redux or serverInfo
rg -n "TxDetailsModal|serverInfo|mapStateToProps" src/components/TxDetailsModal.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 197
🏁 Script executed:
# Get the JSDoc props definition for TxDetailsModal
sed -n '160,180p' src/components/TxDetailsModal.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 566
🏁 Script executed:
# Find where TxDetailsModal is imported and used to see how it receives props
rg -n "TxDetailsModal" src/ --type=js -B2 -A2Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 3665
🏁 Script executed:
# Check MainScreen component to see if it has access to serverInfo
grep -n "serverInfo\|mapStateToProps\|useSelector" src/screens/MainScreen.js | head -30Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 196
🏁 Script executed:
# Check the full props passed to TxDetailsModal in MainScreen
sed -n '104,115p' src/screens/MainScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 351
🏁 Script executed:
# Check MainScreen's mapStateToProps to see if serverInfo is included
sed -n '46,60p' src/screens/MainScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 618
🏁 Script executed:
# Get more context around mapStateToProps
sed -n '40,70p' src/screens/MainScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1100
🏁 Script executed:
# Check how other components access serverInfo when they need it
rg -n "serverInfo" src/screens/ --type=js | grep -i "selector\|mapState" | head -20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 398
🏁 Script executed:
# Check if there's a pattern of passing serverInfo through props in the codebase
rg -B5 -A5 "serverInfo.*=" src/screens/MainScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Check if TxDetailsModal is ever used as a functional component or only as class component
rg -n "function TxDetailsModal\|const TxDetailsModal" src/Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Verify the complete serverInfo.native_token structure once more
rg -B2 -A10 "native_token: {" src/sagas/wallet.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 172
🏁 Script executed:
# Get the native_token structure without regex special chars
sed -n '299,305p' src/sagas/wallet.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 369
🏁 Script executed:
# Check how SendConfirmScreen (another functional component) accesses serverInfo
rg -n "useSelector\|mapStateToProps" src/screens/SendConfirmScreen.js | head -5Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Get SendConfirmScreen component signature
head -50 src/screens/SendConfirmScreen.js | tail -30Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1615
🏁 Script executed:
# Check SendConfirmScreen in full to see how it's a functional component
grep -n "function\|const.*=.*\(\)" src/screens/SendConfirmScreen.js | head -10Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 770
🏁 Script executed:
# Check if SendConfirmScreen uses serverInfo for native symbol or if it's also hardcoded
sed -n '180,200p' src/screens/SendConfirmScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1088
🏁 Script executed:
# Check all locations that use DEFAULT_NATIVE_TOKEN_CONFIG.symbol to see the pattern
rg -n "DEFAULT_NATIVE_TOKEN_CONFIG\.symbol" src/Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 631
🏁 Script executed:
# Check if there are examples of class components being wrapped with Redux connection
rg -n "connect(mapStateToProps" src/screens/ --type=js | head -5Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 160
🏁 Script executed:
# See how other modal/component classes that need Redux get connected
rg -B5 "export default connect" src/screens/ --type=js | head -30Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1198
Use network-provided native symbol for fee labels.
Line 119 uses hardcoded DEFAULT_NATIVE_TOKEN_CONFIG.symbol, which diverges from the connected network's native symbol on custom networks. The serverInfo.native_token.symbol exists in Redux state and should be used instead.
However, TxDetailsModal doesn't currently receive serverInfo as a prop (it only receives token, tx, isNFT, onRequestClose). To fix this, either:
- Pass
serverInfofrom MainScreen to TxDetailsModal, or - Convert TxDetailsModal to a functional component and use
useSelectorto accessserverInfo.native_token.symbol
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/TxDetailsModal.js` at line 119, TxDetailsModal currently reads
the native token symbol from
hathorLib.constants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol (nativeSymbol) which
ignores the connected network; change it to use the network-provided symbol at
serverInfo.native_token.symbol by either passing serverInfo from MainScreen into
TxDetailsModal (add serverInfo to the component props and use
props.serverInfo.native_token.symbol where nativeSymbol is used) or convert
TxDetailsModal into a functional component and call useSelector(state =>
state.server.serverInfo.native_token.symbol) to derive nativeSymbol; update all
references to the old DEFAULT_NATIVE_TOKEN_CONFIG.symbol to use the new value
(retain fallback to DEFAULT_NATIVE_TOKEN_CONFIG.symbol if
serverInfo.native_token.symbol is undefined).
| error: null, | ||
| dirty: false, | ||
| }); | ||
| const network = useSelector((state) => new Network(state.networkSettings.network)); |
There was a problem hiding this comment.
new Network() inside useSelector defeats reference-equality checks and causes spurious re-renders.
useSelector uses === to decide whether to re-render. Because new Network(...) returns a new object reference on every selector invocation, the comparison always fails, so the component re-renders on every Redux dispatch — not just when networkSettings.network changes.
🛠️ Proposed fix
-import React, { useEffect, useRef, useState } from 'react';
+import React, { useEffect, useMemo, useRef, useState } from 'react';- const network = useSelector((state) => new Network(state.networkSettings.network));
+ const networkName = useSelector((state) => state.networkSettings.network);
+ const network = useMemo(() => new Network(networkName), [networkName]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/SendAddressInput.js` at line 52, The current selector creates a
new Network instance on every call which breaks reference-equality and triggers
pointless re-renders; instead, have useSelector return the plain network value
(e.g., state.networkSettings.network) and then create/memoize the Network
instance inside the component using useMemo so it only changes when that
primitive changes—replace the selector that uses new Network(...) with a
selector that returns the raw value and move the Network construction to a
useMemo([networkValue]) block (references: useSelector, useMemo, Network,
networkSettings.network in SendAddressInput.js).
| autoCorrect={false} | ||
| spellCheck={false} | ||
| autoCapitalize='none' | ||
| autoCompleteType='off' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if file exists
ls -la src/screens/SendAddressInput.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 159
🏁 Script executed:
# Look at line 144 in context
sed -n '140,150p' src/screens/SendAddressInput.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 621
🏁 Script executed:
# Check React Native version
cat package.json | grep -A2 '"react-native"'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 801
🌐 Web query:
React Native autoCompleteType vs autoComplete rename breaking change 0.66
💡 Result:
In React Native, the prop rename was from TextInput.autoCompleteType to TextInput.autoComplete. The “autoCompleteType” prop was implemented to map to the old behavior under a new name, and the “autoComplete” prop is the one documented going forward (with “off” supported to disable autocomplete). [1][2][3] What broke around/near 0.66 1) The rename itself was not specific to 0.66. - The discussion/plan to rename autoCompleteType → autoComplete happened in 2019, and the change was implemented via PR #26010 (“Updated TextInput autoCompleteType prop to autoComplete”). [2][1] - The underlying Android native prop was updated so that React prop name autoComplete drives the autofill hints logic, and there’s a deprecated wrapper for autoCompleteType that calls into the same logic (“Delete autoCompleteType prop” TODO + deprecated setter that forwards to setTextContentType(view, autoCompleteType)”). [4] 2) The documentation/modern API is autoComplete, not autoCompleteType. - Current official docs describe autoCompleteand explicitly note that settingautoCompletetooffdisables autocomplete. [3] 3) A common “migration issue” you might have seen after upgrading to a version that started treatingautoCompleteas the canonical prop is using the wrong prop name. - If your code only usedautoCompleteType, newer React Native versions may warn or ignore it depending on which compatibility shims are present in that specific release; the official direction is to use autoComplete. The repo history shows autoCompleteTypewas intentionally transitioned toautoCompleteas the primary prop. [2][4][3] Practical fix - Replace TextInput autoCompleteType with autoComplete. - Before: - <TextInput autoCompleteType="email" /> - After: - <TextInput autoComplete="email" /> - To disable: <TextInput autoComplete="off" /> [3] About “autoComplete vs autoComplete rename breaking change 0.66” - I did not find evidence in the official 0.66 release announcement or 0.66 changelog highlights that the autoCompleteType→autoComplete rename was a new breaking change introduced specifically in 0.66. The rename work is documented as happening earlier (2019/2021 PR activity), andautoComplete` is the documented prop today. [5][2][3]
Citations:
- 1: Rename TextInput prop "autoCompleteType" to "autoComplete" facebook/react-native#26003
- 2: Updated TextInput autoCompleteType prop to autoComplete facebook/react-native#26010
- 3: https://reactnative.dev/docs/textInput
- 4: facebook/react-native@8066bc9ff6a
- 5: https://reactnative.dev/blog/2021/10/01/version-066
autoCompleteType is deprecated and ignored in React Native 0.77 — use autoComplete instead.
The autoCompleteType prop is deprecated in favor of autoComplete. It is silently ignored in React Native 0.77.2, so the autocomplete suppression has no effect on Android with the current code. The official API documentation lists only autoComplete.
🛠️ Proposed fix
- autoCompleteType='off'
+ autoComplete='off'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| autoCompleteType='off' | |
| autoComplete='off' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/SendAddressInput.js` at line 144, Replace the deprecated prop
autoCompleteType with the supported autoComplete on the TextInput in
SendAddressInput.js: find the TextInput (or the component rendering address
input where autoCompleteType='off' is set) and change it to autoComplete='off'
so React Native 0.77 will honor the autocomplete suppression; keep the same
placement and any other props/handlers (e.g., value, onChangeText) intact.
Four post-PR fixes from Yan's review pass on the shielded-outputs
send flow:
1. SendAddressInput / SendAmountInput — `keyboardVerticalOffset`
- Both screens were passing `getStatusBarHeight()` to
`<KeyboardAvoidingView>`, but the KAV is rendered after the
`<HathorHeader />` in the JSX, so its top sits at
`statusBar + headerHeight` from the screen top. With the
under-stated offset KAV computed too little bottom padding when
the keyboard appeared, and on first focus the NEXT button ended
up flush with the keyboard top — clickable only after the user
dismissed the keyboard once and reopened it (the iOS
keyboard-show remeasure happened to land in the right place the
second time). Switched both screens to
`getKeyboardAvoidingViewTopDistance()` (status bar + header).
- Restructured SendAddressInput to mirror SendAmountInput's
layout (Pressable wraps both header and KAV, single flex
container inside KAV) so the same fix applies to both screens
and so the "NEXT in the middle of the screen after returning
from Token Swap" bug Yan also reported is resolved.
2. PrivacyModeCard — Switch off-state visibility
- Off-track was using `COLORS.borderColor` (#eee) on a #FAFAFA
card, which Yan reported as "almost invisible". An aggressive
#D1D5DB darker gray fixed visibility but split the track
visually with the white thumb. Settled on #DCDCDC: enough
contrast to outline the off-state without making the thumb pop
as a high-contrast accent. `ios_backgroundColor` left to its
default so iOS's transition animation doesn't double up the
contrast.
3. Settings — Reown moved from Advanced to General
- Slotted between Security and Push Notification so it reads as
a wallet-feature-level item alongside the other things users
interact with day-to-day, instead of being grouped with
network/privacy/reset.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/SendAmountInput.js (1)
229-249:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid hardcoding "HTR" in user-facing error strings.
The error messages on lines 231, 236, and 247 hardcode "HTR" as the native token symbol. Based on learnings, user-facing strings should pull the native symbol from
serverInfo.native_token.symbol(withconstants.NATIVE_TOKEN_UIDas a temporary fallback) so they reflect the connected fullnode and stay consistent across screens. The component already readsserverInfo(line 50) fordecimal_places, so extending that selector is straightforward.🛠️ Suggested fix
- const { decimalPlaces } = useSelector((state) => ({ - decimalPlaces: state.serverInfo?.decimal_places - })); + const { decimalPlaces, nativeSymbol } = useSelector((state) => ({ + decimalPlaces: state.serverInfo?.decimal_places, + nativeSymbol: state.serverInfo?.native_token?.symbol, + }));if (!htrBalance) { - setError(t`Insufficient balance of HTR to cover the network fee.`); + setError(t`Insufficient balance of ${nativeSymbol} to cover the network fee.`); return; } const { available: htrAvailable } = htrBalance.data; if (networkFee > htrAvailable) { - setError(t`Insufficient balance of HTR to cover the network fee.`); + setError(t`Insufficient balance of ${nativeSymbol} to cover the network fee.`); return; } } else if (privacyMode !== 'public' && networkFee > 0n) { ... if (htrAvailable < required) { - setError(t`Insufficient HTR balance to cover the shielded transaction fee.`); + setError(t`Insufficient ${nativeSymbol} balance to cover the shielded transaction fee.`); return; }Based on learnings: "Do not hardcode the native token name or symbol in any user-facing UI ... use serverInfo.native_token.symbol for the symbol ... This applies to all Text components, labels, tooltips, and similar UI elements across the app."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/SendAmountInput.js` around lines 229 - 249, Replace hardcoded "HTR" in user-facing error messages inside SendAmountInput.js with the native token symbol from serverInfo (use serverInfo.native_token.symbol and fall back to hathorLib.constants.NATIVE_TOKEN_UID or a default symbol if missing); update the three setError(...) calls that currently embed "HTR" (the checks around tokensBalance lookup with hathorLib.constants.NATIVE_TOKEN_UID and the shielded-fee branch using isHTR and htrAvailable) to construct the message using the resolved nativeSymbol variable so the UI shows the connected fullnode's native token symbol.
♻️ Duplicate comments (2)
src/screens/SendAddressInput.js (2)
161-161:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace deprecated
autoCompleteTypewithautoComplete(still unresolved).Line 161 uses a deprecated prop; on current React Native versions this can be ignored, so disabling autocomplete may not work reliably.
🛠️ Proposed fix
- autoCompleteType='off' + autoComplete='off'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/SendAddressInput.js` at line 161, Replace the deprecated prop autoCompleteType with the newer autoComplete in the SendAddressInput component: locate where autoCompleteType='off' (used on the TextInput inside SendAddressInput) and change it to autoComplete='off' (or remove it if you want native default behavior), ensuring any platform-specific fallback logic around the TextInput render path is updated to use autoComplete and that prop validation/Flow/TS types (if any) reference the new prop name.
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove
Networkconstruction out ofuseSelectorto avoid spurious re-renders (still unresolved).Line 52 still returns a new object on every selector run, so
useSelector’s===check keeps failing on unrelated Redux dispatches.🛠️ Proposed fix
-import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useMemo, useRef, useState } from 'react'; ... - const network = useSelector((state) => new Network(state.networkSettings.network)); + const networkName = useSelector((state) => state.networkSettings.network); + const network = useMemo(() => new Network(networkName), [networkName]);Also applies to: 52-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/SendAddressInput.js` at line 8, The selector is creating a new Network instance on every run causing useSelector to see a different object; move the Network construction out of the selector in SendAddressInput (e.g., create the Network once at module scope or inside the SendAddressInput component using useRef/useMemo) and have useSelector return only primitive/serializable pieces (or the same stable reference) instead of new Network(...); update any code referencing the selector to use the stable Network reference (Network constructor call, the selector that currently returns Network) so repeated Redux dispatches no longer trigger unnecessary re-renders.
🧹 Nitpick comments (3)
src/screens/Settings.js (2)
133-139: 💤 Low valueConsider wrapping the Reown label in
t\...`` for translation consistency.Every other entry in this screen uses ttag (
t\Security`,t`Push Notification``, etc.). Even though "Reown" is a brand name and likely won't be translated, wrapping it keeps the extraction pipeline consistent and avoids being the only odd-one-out string in the menu. Skip this if the team has a deliberate convention of leaving brand names unwrapped.♻️ Optional consistency tweak
{this.props.reownEnabled && ( <ListMenu - title='Reown' + title={t`Reown`} onPress={() => this.props.navigation.navigate('ReownList')} /> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/Settings.js` around lines 133 - 139, The Reown menu title is the only hard-coded label—wrap the title string used in the ListMenu component (title currently 'Reown' in the JSX where this.props.reownEnabled controls rendering) with the ttag template (t`Reown`) to match other entries like t`Security` and t`Push Notification`, keeping the translation extraction pipeline consistent; update the title prop passed to ListMenu accordingly.
113-147: 💤 Low valueReown reorg in GENERAL SETTINGS looks correct; minor
isFirstedge case worth confirming.The Reown entry is correctly placed between Security and Push Notification per the PR objective, and conditional rendering is unchanged. One small observation:
isFirstis hardcoded on the "Register a token" item (line 119), so ifIS_MULTI_TOKENis everfalse, both token entries are skipped and "Security" becomes the visible first row withoutisFirst, which would drop the top rounded corners on this card. IfIS_MULTI_TOKENis a static build-time constant that is alwaystrue, you can ignore this.♻️ Optional fallback for the `isFirst` flag
{IS_MULTI_TOKEN && ( <ListMenu title={t`Register a token`} onPress={() => this.props.navigation.navigate('RegisterToken')} isFirst /> )} {IS_MULTI_TOKEN && ( <ListMenu title={t`Create a new token`} onPress={() => this.props.navigation.navigate('CreateTokenStack')} /> )} <ListMenu title={t`Security`} onPress={() => this.props.navigation.navigate('Security')} + isFirst={!IS_MULTI_TOKEN} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/Settings.js` around lines 113 - 147, The ListMenu "Register a token" hardcodes isFirst which means when IS_MULTI_TOKEN is false the first visible item ("Security") won't get isFirst and the card's top corners may not be rounded; update the rendering logic in Settings.js so the first visible ListMenu gets isFirst dynamically (e.g. compute a boolean like isFirstItem for the first rendered menu or set isFirst on the "Security" ListMenu when IS_MULTI_TOKEN is false) — adjust inside the HathorList block where ListMenu components (RegisterToken, CreateTokenStack, Security, Reown, PushNotification) are conditionally rendered so the top-most visible ListMenu receives the isFirst prop.src/screens/SendAmountInput.js (1)
333-374: ⚡ Quick winExtract the privacy-mode display metadata to share with
TransactionPrivacyModal.The inline IIFE picks an icon, title, and subtitle for each privacy mode and the comment notes the same SVG/copy is "Same SVG components and copy used there, kept centralised". In practice, this metadata is duplicated between
SendAmountInputandTransactionPrivacyModal, so a label tweak today still requires editing both files. Extracting a small helper (e.g.getPrivacyModeMetadata(mode)) into a shared module would actually achieve the centralisation the comment describes and trim this render block.♻️ Sketch
// e.g. src/components/privacyModeMetadata.js import { ShieldPadlockIcon } from './Icons/ShieldPadlock.icon'; import { EyeOffSlashIcon } from './Icons/EyeOffSlash.icon'; import { EyeIcon } from './Icons/Eye.icon'; import { PRIVACY_MODE } from '../constants'; import { t } from 'ttag'; export const getPrivacyModeMetadata = (mode) => { switch (mode) { case PRIVACY_MODE.PRIVATE: return { Icon: ShieldPadlockIcon, title: t`Private`, subtitle: t`Token and amount hidden` }; case PRIVACY_MODE.HIDE_AMOUNT: return { Icon: EyeOffSlashIcon, title: t`Hide amount`, subtitle: t`Token visible, amount hidden` }; default: return { Icon: EyeIcon, title: t`Public`, subtitle: t`Token and amount visible` }; } };Then both
SendAmountInputandTransactionPrivacyModalconsume the same source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/SendAmountInput.js` around lines 333 - 374, The render block in SendAmountInput duplicates privacy-mode icon/title/subtitle logic that is also used in TransactionPrivacyModal; extract this into a shared helper like getPrivacyModeMetadata(mode) and import it into both SendAmountInput and TransactionPrivacyModal. Implement getPrivacyModeMetadata to accept the privacyMode (or PRIVACY_MODE constant) and return { Icon, title, subtitle } using the existing SVG components (ShieldPadlockIcon, EyeOffSlashIcon, EyeIcon) and t`` strings, then replace the inline IIFE in SendAmountInput (the block that reads privacyMode and sets ModeIcon/modeTitle/modeSubtitle) to call the helper and use the returned Icon/title/subtitle.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/screens/SendAmountInput.js`:
- Around line 146-152: The catch in the getUtxosForAmount flow leaves utxos=null
for transparent non-FEE tokens which allows navigation to SendConfirmScreen;
update the error handling so the app blocks navigation and surfaces an error: in
the catch block inside the async fetch (where getUtxosForAmount is called and
setNetworkFee is used), set a new error state (e.g. setUtxosFetchError or
setUtxosError) when the call fails for transparent/public tokens (privacyMode
=== 'public' and token !== FEE token) and keep utxos as null; then extend the
existing guard that checks expectsFee (the logic around expectsFee/navigation
check) to also prevent navigation when utxosFetchError is set (or alternately
add a check for privacyMode==='public' && utxos==null) so users cannot proceed
and see an explanatory error message instead of losing their UTXO selection.
---
Outside diff comments:
In `@src/screens/SendAmountInput.js`:
- Around line 229-249: Replace hardcoded "HTR" in user-facing error messages
inside SendAmountInput.js with the native token symbol from serverInfo (use
serverInfo.native_token.symbol and fall back to
hathorLib.constants.NATIVE_TOKEN_UID or a default symbol if missing); update the
three setError(...) calls that currently embed "HTR" (the checks around
tokensBalance lookup with hathorLib.constants.NATIVE_TOKEN_UID and the
shielded-fee branch using isHTR and htrAvailable) to construct the message using
the resolved nativeSymbol variable so the UI shows the connected fullnode's
native token symbol.
---
Duplicate comments:
In `@src/screens/SendAddressInput.js`:
- Line 161: Replace the deprecated prop autoCompleteType with the newer
autoComplete in the SendAddressInput component: locate where
autoCompleteType='off' (used on the TextInput inside SendAddressInput) and
change it to autoComplete='off' (or remove it if you want native default
behavior), ensuring any platform-specific fallback logic around the TextInput
render path is updated to use autoComplete and that prop validation/Flow/TS
types (if any) reference the new prop name.
- Line 8: The selector is creating a new Network instance on every run causing
useSelector to see a different object; move the Network construction out of the
selector in SendAddressInput (e.g., create the Network once at module scope or
inside the SendAddressInput component using useRef/useMemo) and have useSelector
return only primitive/serializable pieces (or the same stable reference) instead
of new Network(...); update any code referencing the selector to use the stable
Network reference (Network constructor call, the selector that currently returns
Network) so repeated Redux dispatches no longer trigger unnecessary re-renders.
---
Nitpick comments:
In `@src/screens/SendAmountInput.js`:
- Around line 333-374: The render block in SendAmountInput duplicates
privacy-mode icon/title/subtitle logic that is also used in
TransactionPrivacyModal; extract this into a shared helper like
getPrivacyModeMetadata(mode) and import it into both SendAmountInput and
TransactionPrivacyModal. Implement getPrivacyModeMetadata to accept the
privacyMode (or PRIVACY_MODE constant) and return { Icon, title, subtitle }
using the existing SVG components (ShieldPadlockIcon, EyeOffSlashIcon, EyeIcon)
and t`` strings, then replace the inline IIFE in SendAmountInput (the block that
reads privacyMode and sets ModeIcon/modeTitle/modeSubtitle) to call the helper
and use the returned Icon/title/subtitle.
In `@src/screens/Settings.js`:
- Around line 133-139: The Reown menu title is the only hard-coded label—wrap
the title string used in the ListMenu component (title currently 'Reown' in the
JSX where this.props.reownEnabled controls rendering) with the ttag template
(t`Reown`) to match other entries like t`Security` and t`Push Notification`,
keeping the translation extraction pipeline consistent; update the title prop
passed to ListMenu accordingly.
- Around line 113-147: The ListMenu "Register a token" hardcodes isFirst which
means when IS_MULTI_TOKEN is false the first visible item ("Security") won't get
isFirst and the card's top corners may not be rounded; update the rendering
logic in Settings.js so the first visible ListMenu gets isFirst dynamically
(e.g. compute a boolean like isFirstItem for the first rendered menu or set
isFirst on the "Security" ListMenu when IS_MULTI_TOKEN is false) — adjust inside
the HathorList block where ListMenu components (RegisterToken, CreateTokenStack,
Security, Reown, PushNotification) are conditionally rendered so the top-most
visible ListMenu receives the isFirst prop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dabf1e8d-5367-43f2-82b4-9114b5f6ef2e
📒 Files selected for processing (4)
src/components/PrivacyModeCard.jssrc/screens/SendAddressInput.jssrc/screens/SendAmountInput.jssrc/screens/Settings.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/PrivacyModeCard.js
| } catch (err) { | ||
| if (!cancelled) { | ||
| setNetworkFee(hathorLib.constants.FEE_PER_OUTPUT); | ||
| const tokenFeeMultiplier = privacyMode !== 'public' ? 2n : 1n; | ||
| setNetworkFee(tokenFeeMultiplier * hathorLib.constants.FEE_PER_OUTPUT + shieldedFee); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether SendConfirmScreen tolerates utxos === null on the transparent path.
fd -t f 'SendConfirmScreen.js' -x rg -nP -C5 '\butxos\b'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1153
🏁 Script executed:
#!/bin/bash
# Get context around lines 125, 146-152, and 222 in SendAmountInput.js
fd -t f 'SendAmountInput.js' | head -1 | xargs -I {} sh -c '
echo "=== Lines 120-130 (reset context) ==="
sed -n "120,130p" "{}"
echo ""
echo "=== Lines 141-160 (catch block and surroundings) ==="
sed -n "141,160p" "{}"
echo ""
echo "=== Lines 215-230 (guard mentioned in review) ==="
sed -n "215,230p" "{}"
'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 2038
🏁 Script executed:
#!/bin/bash
# Understand the full context of the utxos variable and onButtonPress logic
fd -t f 'SendAmountInput.js' | head -1 | xargs -I {} sh -c '
echo "=== Full function handling utxos and networkFee ==="
rg -n "const onButtonPress|networkFee|utxos" "{}" -B2 -A2 | head -100
'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 2026
🏁 Script executed:
#!/bin/bash
# Check SendConfirmScreen usage of inputs to understand the impact
fd -t f 'SendConfirmScreen.js' | head -1 | xargs -I {} sh -c '
echo "=== How inputs are used in SendConfirmScreen ==="
rg -n "inputs" "{}" -B2 -A2 | head -80
'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1378
Transparent non-FEE transactions can bypass the fee calculation guard when getUtxosForAmount fails.
When wallet.getUtxosForAmount throws on a transparent non-FEE token transaction, the catch block sets a fallback networkFee but leaves utxos as null. The guard at line 222 only prevents navigation for FEE tokens or shielded transactions (where expectsFee = true); for transparent non-FEE tokens, it does not apply, allowing the user to proceed to SendConfirmScreen with utxos: null. While SendConfirmScreen safely converts null to an empty array via the ternary at line 285–287, passing empty inputs to wallet.sendManyOutputsSendTransaction() means the wallet re-selects UTXOs instead of reusing the pre-committed selection, silently losing the user's prior UTXO choice when the network request fails.
Consider either extending the guard at line 222 to also cover transparent non-FEE tokens, or setting an error state in the catch block so the user understands why the action is blocked.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/SendAmountInput.js` around lines 146 - 152, The catch in the
getUtxosForAmount flow leaves utxos=null for transparent non-FEE tokens which
allows navigation to SendConfirmScreen; update the error handling so the app
blocks navigation and surfaces an error: in the catch block inside the async
fetch (where getUtxosForAmount is called and setNetworkFee is used), set a new
error state (e.g. setUtxosFetchError or setUtxosError) when the call fails for
transparent/public tokens (privacyMode === 'public' and token !== FEE token) and
keep utxos as null; then extend the existing guard that checks expectsFee (the
logic around expectsFee/navigation check) to also prevent navigation when
utxosFetchError is set (or alternately add a check for privacyMode==='public' &&
utxos==null) so users cannot proceed and see an explanatory error message
instead of losing their UTXO selection.
Adds two rows to the bottom of the transaction-detail modal that let
the user share the per-tx unblinding payload with an auditor (or
themselves on a desktop browser):
- "View in Explorer unblinded" — opens the explorer at the tx page
with the payload in the URL fragment (`#unblind=…`). Fragments
are not sent to any server, so the payload only ever exists on
the recipient's client.
- "Copy unblinding values" — copies the same base64url-encoded
payload to the clipboard for users who don't want secrets in
their address bar / shell history.
The new component (`AuditUnblindingRows`) calls wallet-lib's
`HathorWallet.getShieldedUnblindingForTx(txId)` (added in
0.0.8-shielded) which returns openings only for shielded outputs the
wallet OWNS (received + change) and inputs whose parent output the
wallet owned. Outputs the wallet generated for other recipients are
deliberately excluded — privacy correctness, the wallet has no
business disclosing someone else's blinding factors. So both rows
hide entirely on a tx where the wallet has nothing to share (e.g.
an outgoing-only tx with no change back).
Metro config:
- Adds Node-stdlib shims (assert / buffer / console / crypto / path
/ stream / zlib) at the resolver level. Mobile already had these
aliased in package.json's `react-native` field, but that field
only applies to the mobile app's OWN source — Metro doesn't
propagate it into deeply-resolved deps. With wallet-lib copied
into node_modules (rather than symlinked, see release script
below), Metro's resolver suddenly walks into wallet-lib's deep
`require('path')` / `require('crypto')` calls and needs the
shims to be globally visible.
- Adds an empty-module shim for `@hathor/ct-crypto-wasm`. wallet-lib
0.0.8-shielded ships a browser-only `provider.browser.js` that
does `await import('@hathor/ct-crypto-wasm')`. Mobile never
reaches that code (it uses the NAPI provider via ct-crypto-node),
but Metro statically analyzes dynamic imports — without the shim
the bundle fails. Same pattern as the pre-existing
ct-crypto-node shim.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…patch - @hathor/wallet-lib: file:../hathor-wallet-lib → 0.0.8-shielded (exact pin per repo convention — no ^ / ~). Brings the `getShieldedUnblindingForTx` API + verifier crypto primitives + schema relaxation for legacy fullnodes that still emit shielded outputs without `mode` / `token_data`. - patches/@Hathor+wallet-lib+0.0.6-shielded.patch: removed. The only behaviorally-significant hunk (FullShielded `tokens[]` privacy guard in `sendTransaction.js`) is upstreamed in 0.0.7+; the remaining hunks were sourcemap deltas + a comment. - patches/@Hathor+wallet-lib+0.0.8-shielded.patch: regenerated by `npx patch-package @hathor/wallet-lib`. Carries the Hermes/JSC bigInt-reviver compat fix (early-return when `context` is undefined, plus broader SyntaxError matching for JSC's "can't convert" and Hermes's "Failed to parse String to BigInt"). The fix has lived in patches/ at every wallet-lib bump and is not going upstream — see the recurring task in the team's notes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
package.json (1)
3-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVersion regression breaks CI and will likely break store uploads.
The pipeline is explicitly reporting:
Version mismatch: package.json is 0.0.1-shielded but android/app/build.gradle differs.This is the same root-cause version regression that was already flagged — update to0.38.0-shielded(or align with the iOSMARKETING_VERSION) to fix both the CI failure and prevent store-upload rejection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 3, The package.json "version" field has regressed to "0.0.1-shielded", causing CI/version mismatch with android/app/build.gradle and store-upload failures; update the "version" value in package.json (the "version" key) to the correct release string "0.38.0-shielded" (or otherwise match the iOS MARKETING_VERSION) so the project versions are aligned across package.json, android/app/build.gradle and iOS build settings.
🧹 Nitpick comments (3)
patches/@Hathor+wallet-lib+0.0.8-shielded.patch (1)
26-34: 💤 Low valueConsider tightening the
"can't convert"substring match.
e.message.indexOf("can't convert") !== -1will match anySyntaxErrorwhose message contains that phrase — not only BigInt conversion failures. The JSC message is reportedly"can't convert string to bigint", soe.message.includes("can't convert") && e.message.includes("bigint")(or a similar combined check) would be more defensive without meaningfully increasing complexity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/`@hathor+wallet-lib+0.0.8-shielded.patch around lines 26 - 34, The current SyntaxError message check in the thrown-error branch (the conditional using e instanceof SyntaxError and e.message.indexOf("can't convert") !== -1) is too broad; tighten it by requiring both the "can't convert" substring and a BigInt indicator (e.g., "bigint" or "BigInt") appear in e.message (use e.message.includes("can't convert") && e.message.toLowerCase().includes("bigint") or equivalent) so the clause only matches JSC's "can't convert string to bigint" cases without catching unrelated SyntaxErrors.src/components/AuditUnblindingRows.js (2)
149-149: 💤 Low valueUnhandled
Linking.openURLrejection.
Linking.openURLreturns a Promise that rejects when the URL cannot be opened (e.g., no browser installed, malformed URL). The rejection is silently swallowed here. Consider adding a.catchto surface a user-facing error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AuditUnblindingRows.js` at line 149, The onPress handler in AuditUnblindingRows calls Linking.openURL(explorerLink) without handling its rejected Promise; update the handler to guard and handle failures by using Linking.canOpenURL(explorerLink) before opening and/or appending a .catch on Linking.openURL to surface a user-facing error (e.g., Alert.alert or toast) and log the error; ensure you reference the explorerLink variable and the onPress callback so the UI shows a clear message when opening the URL fails.
130-134: ⚡ Quick win
setTimeouthandle not cleared on unmount — can set state on a stale component.If the component unmounts within the 1500 ms window, the
setTimeoutcallback fires and callssetCopying(false)on the already-unmounted instance. React 18 dropped the warning, but it's still a leak.♻️ Proposed fix
- const onCopy = () => { - Clipboard.setString(payload); - setCopying(true); - setTimeout(() => setCopying(false), 1500); - }; + const copyTimerRef = React.useRef(null); + const onCopy = () => { + Clipboard.setString(payload); + setCopying(true); + if (copyTimerRef.current) clearTimeout(copyTimerRef.current); + copyTimerRef.current = setTimeout(() => setCopying(false), 1500); + };And clear it in the effect cleanup (or a dedicated
useEffect):+ useEffect(() => () => { + if (copyTimerRef.current) clearTimeout(copyTimerRef.current); + }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AuditUnblindingRows.js` around lines 130 - 134, The onCopy handler uses setTimeout to clear setCopying(false) but never clears that timer on unmount; update onCopy to store the timeout id (e.g., in a timerRef via useRef) and clear any existing timeout before setting a new one, and add a useEffect cleanup that calls clearTimeout(timerRef.current) to avoid calling setCopying on an unmounted component; reference the onCopy function, setCopying state setter, Clipboard.setString call, and the timeout id stored in timerRef.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/AuditUnblindingRows.js`:
- Line 139: The explorerIcon uses React Native's Image with width/height passed
as direct props which are ignored; change the JSX so Image (the component used
to render icShareActive) receives width and height via a style object (e.g.,
style={{width: 24, height: 24}}) or a named style constant, ensuring the
rendered share icon is sized to 24x24; update the explorerIcon definition that
currently references Image and icShareActive accordingly.
- Line 9: The current import in AuditUnblindingRows.js pulls Clipboard from
'react-native' which is removed; install the community package
(`@react-native-clipboard/clipboard`) and replace the import so Clipboard is
imported from '@react-native-clipboard/clipboard' while keeping Image and
Linking from 'react-native'; ensure the call site (Clipboard.setString used in
AuditUnblindingRows, e.g. where Clipboard.setString(payload) is invoked) still
calls the package's API and update any references if needed.
---
Duplicate comments:
In `@package.json`:
- Line 3: The package.json "version" field has regressed to "0.0.1-shielded",
causing CI/version mismatch with android/app/build.gradle and store-upload
failures; update the "version" value in package.json (the "version" key) to the
correct release string "0.38.0-shielded" (or otherwise match the iOS
MARKETING_VERSION) so the project versions are aligned across package.json,
android/app/build.gradle and iOS build settings.
---
Nitpick comments:
In `@patches/`@hathor+wallet-lib+0.0.8-shielded.patch:
- Around line 26-34: The current SyntaxError message check in the thrown-error
branch (the conditional using e instanceof SyntaxError and
e.message.indexOf("can't convert") !== -1) is too broad; tighten it by requiring
both the "can't convert" substring and a BigInt indicator (e.g., "bigint" or
"BigInt") appear in e.message (use e.message.includes("can't convert") &&
e.message.toLowerCase().includes("bigint") or equivalent) so the clause only
matches JSC's "can't convert string to bigint" cases without catching unrelated
SyntaxErrors.
In `@src/components/AuditUnblindingRows.js`:
- Line 149: The onPress handler in AuditUnblindingRows calls
Linking.openURL(explorerLink) without handling its rejected Promise; update the
handler to guard and handle failures by using Linking.canOpenURL(explorerLink)
before opening and/or appending a .catch on Linking.openURL to surface a
user-facing error (e.g., Alert.alert or toast) and log the error; ensure you
reference the explorerLink variable and the onPress callback so the UI shows a
clear message when opening the URL fails.
- Around line 130-134: The onCopy handler uses setTimeout to clear
setCopying(false) but never clears that timer on unmount; update onCopy to store
the timeout id (e.g., in a timerRef via useRef) and clear any existing timeout
before setting a new one, and add a useEffect cleanup that calls
clearTimeout(timerRef.current) to avoid calling setCopying on an unmounted
component; reference the onCopy function, setCopying state setter,
Clipboard.setString call, and the timeout id stored in timerRef.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c448205-8cc7-4400-849e-fb19fe0c0c5d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
metro.config.jspackage.jsonpatches/@hathor+wallet-lib+0.0.8-shielded.patchsrc/components/AuditUnblindingRows.jssrc/components/TxDetailsModal.js
🚧 Files skipped from review as they are similar to previous changes (2)
- metro.config.js
- src/components/TxDetailsModal.js
| // Share/external-link glyph reserved for the row that hands off to | ||
| // mobile Safari; the copy row uses FontAwesome's `faCopy` to read as | ||
| // "writes to clipboard" rather than "opens elsewhere". | ||
| const explorerIcon = <Image source={icShareActive} width={24} height={24} />; |
There was a problem hiding this comment.
width and height as direct props on <Image> are silently ignored — use style.
React Native's Image component does not accept width/height as direct layout props; it uses the style prop to define the width and height of the image. Passing them as top-level props has no effect, so the share icon will render at its intrinsic/natural asset size rather than the intended 24 dp.
🐛 Proposed fix
- const explorerIcon = <Image source={icShareActive} width={24} height={24} />;
+ const explorerIcon = <Image source={icShareActive} style={{ width: 24, height: 24 }} />;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const explorerIcon = <Image source={icShareActive} width={24} height={24} />; | |
| const explorerIcon = <Image source={icShareActive} style={{ width: 24, height: 24 }} />; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/AuditUnblindingRows.js` at line 139, The explorerIcon uses
React Native's Image with width/height passed as direct props which are ignored;
change the JSX so Image (the component used to render icShareActive) receives
width and height via a style object (e.g., style={{width: 24, height: 24}}) or a
named style constant, ensuring the rendered share icon is sized to 24x24; update
the explorerIcon definition that currently references Image and icShareActive
accordingly.
Two changes on the Settings → Privacy screen:
1. New "Export privacy keys" row (under a new AUDIT section) opens a
PIN-gated screen that derives and reveals two BIP32 keys an
auditor needs to find + decrypt every shielded output this wallet
receives:
- Spend account xpub at m/44'/280'/2'/0 (read-only — enumerates
every spend-side P2PKH receiving address; CANNOT sign)
- Scan account xpriv at m/44'/280'/1'/0 (read-only by
construction — used only for ECDH range-proof rewind, NEVER
for signing)
Together these grant zero spending authority over either shielded
UTXOs (need account 2 xpriv, not exposed) or transparent funds
(need account 0 xpriv, not exposed). The new screen surfaces a
warning card explaining this before revealing the keys, with
per-key reveal toggle (default masked) and copy-to-clipboard.
Wallet-lib provides the PIN-gated derivation as
`wallet.storage.getSpendXPubKey()` and
`wallet.storage.getScanXPrivKey(pin)`. The flow gates entry
behind PinScreen with a callback that hands the PIN to
ExportPrivacyKeys; the PIN is read once on mount and falls out of
memory when the screen unmounts (route params get GC'd with the
popped frame).
2. Default-privacy-mode preferences screen now persists each toggle
change immediately rather than waiting for an explicit "Save
Preferences" button. Removed the footer button + the local
draft-vs-persisted state split — `selectedMode` reads directly
from Redux and `onSelect` dispatches `privacyDefaultModeSet` with
a no-op guard if the same mode is re-tapped. Trims the export
row to label-only with a chevron-right affordance, matching the
"settings list row" style used by HathorList elsewhere.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumps both Debug and Release configurations for the next TestFlight build that ships the per-tx unblinding flow + Export privacy keys features. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
ios/HathorMobile.xcodeproj/project.pbxproj (1)
489-489:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPipeline confirms
MARKETING_VERSION = 0.38.99999must matchpackage.jsonbefore merge.CI's
check_versionjob is failing becausepackage.jsonis0.0.1-shieldedwhileMARKETING_VERSIONis0.38.99999(both occurrences at lines 489 and 524). Align these to the actual release version before merging tomaster.Also applies to: 524-524
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/HathorMobile.xcodeproj/project.pbxproj` at line 489, The MARKETING_VERSION entries in the Xcode project (symbol MARKETING_VERSION in project.pbxproj) are set to 0.38.99999 but must match package.json (currently 0.0.1-shielded) to satisfy the CI check_version job; update both MARKETING_VERSION occurrences to the exact version string from package.json (or update package.json if intended to change the release version), verify both occurrences (the two MARKETING_VERSION entries) are identical, and re-run the check_version CI job before merging.
🧹 Nitpick comments (1)
patches/@Hathor+wallet-lib+0.0.9-shielded.patch (1)
26-34: 💤 Low valueSubstring match on
"can't convert"is overly broad.
e.message.indexOf("can't convert") !== -1will swallow any futureSyntaxErrorwhose message happens to contain that fragment, not just the documented JSC"can't convert string to bigint". Tightening the match reduces the risk of masking unrelated bugs. Also,String.prototype.includesreads more naturally thanindexOf(...) !== -1.♻️ Suggested tightening
- e.message.indexOf("can't convert") !== -1 || - e.message.indexOf('Failed to parse String to BigInt') !== -1 + e.message.includes("can't convert string to bigint") || + e.message.includes('Failed to parse String to BigInt')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/`@hathor+wallet-lib+0.0.9-shielded.patch around lines 26 - 34, The current conditional in the if block that checks e.message uses a broad substring match e.message.indexOf("can't convert") which can mask unrelated SyntaxErrors; tighten this by matching the specific JSC message (e.g., "can't convert string to bigint") instead of the fragment and use String.prototype.includes or an exact equality/regex to match that full phrase, and replace the indexOf('Failed to parse String to BigInt') similarly with includes; update the conditional around the SyntaxError check (the block referencing e, context?.source and the BigInt message comparisons) to use these narrower comparisons.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/HathorMobile.xcodeproj/project.pbxproj`:
- Line 292: The pbxproj contains a hardcoded Nix store path assigned to
NODE_BINARY in the shellScript entry which will break builds on machines without
that exact store path; remove the fixed export line (the export NODE_BINARY=...
assignment) from the shellScript that runs
../node_modules/react-native/scripts/react-native-xcode.sh so the script can
resolve node from PATH, or alternatively make it read from an external env/user
setting by exporting NODE_BINARY only if not already set (i.e. respect an
existing $NODE_BINARY) before invoking react-native-xcode.sh; update the
shellScript entry accordingly to stop committing a fixed Nix store path.
- Line 475: The CURRENT_PROJECT_VERSION entry `CURRENT_PROJECT_VERSION = 0.7.0;`
does not match the project's version-validation patterns; update the
package.json version and/or the CURRENT_PROJECT_VERSION to a matching format:
either change package.json from `0.0.1-shielded` to a numeric RC like
`0.0.1-rc1` (which will require CURRENT_PROJECT_VERSION to follow `0.1.X`), or
make this a release by setting package.json to `1.0.0` and then set
CURRENT_PROJECT_VERSION to the release pattern `1.0.X`; ensure the two values
align with the check_version script rules so CI validation passes.
In `@patches/`@hathor+wallet-lib+0.0.9-shielded.patch:
- Around line 9-17: The reviver fallback returns JS Numbers when `context` is
undefined, causing silent precision loss for unsafe-range integers on
Hermes/JSC; locate the reviver (references: `context`, `value`) and ensure every
network-parsed numeric field is validated through the BigInt-aware schema
`bigIntCoercibleSchema` (or routed to a BigInt-aware parsing path) before use —
audit and update all call sites (at minimum `src/utils.js` and
`src/components/Reown/NanoContract/BaseNanoContractRequest.js`) to run that
validation, add explicit runtime checks that error/log if a numeric value
exceeds Number.MAX_SAFE_INTEGER when `context` is missing, and add brief
documentation noting `USE_HERMES = true` implications so callers know to use the
BigInt-aware path.
In `@src/screens/ExportPrivacyKeys.js`:
- Around line 43-47: The clipboard copy handler onCopy currently calls
Clipboard.setString(value) and only sets a UI "copied" flag for 1.5s, leaving
sensitive xpriv in the OS clipboard indefinitely; update onCopy to schedule an
automated clipboard wipe (e.g., call Clipboard.setString('') or an
OS-appropriate clear API) after a short window such as 60s using a timeout and
ensure that any pending wipe is cleared if the component unmounts; also update
the warning card text in ExportPrivacyKeys to document this trade-off and the
chosen wipe delay so users are informed.
- Around line 9-17: The Clipboard symbol is imported from react-native core
(e.g., in ExportPrivacyKeys.js and components CopyClipboard.js,
AuditUnblindingRows.js, Reown/TokenInfoModal.js,
Reown/SendTransactionRequest.js) but is removed in RN 0.77; replace those
imports by importing Clipboard from '@react-native-clipboard/clipboard' and keep
using the existing API (e.g., Clipboard.setString(text)); update any
default/named import forms to match the new package, run a quick grep for "from
'react-native' ... Clipboard" to find all occurrences and update them, and run
the app/linter to ensure no remaining undefined Clipboard usages remain.
- Around line 106-211: ExportPrivacyKeys currently displays highly sensitive
keys without preventing screenshots/recents/recordings; fix by enabling
screen-capture protection on mount and disabling on unmount (use useEffect in
ExportPrivacyKeys to call a screen-security library such as
react-native-screen-capture-secure or react-native-screenshot-prevent, which
sets Android FLAG_SECURE and applies iOS blurring/capture listeners), and apply
the same mount/unmount protection in PinScreen so the PIN entry and the path
that navigates to ExportPrivacyKeys are also protected; ensure you call the
library's enable/disable (or equivalent) APIs, and attach any iOS
capturedDidChange/applicationWillResignActive handlers for blur/warning while
the screen is active.
- Around line 107-114: ExportPrivacyKeys currently reads the raw PIN from route
params via useParams (pin) which exposes sensitive data; instead derive
spendXpub and scanXpriv inside the PinScreen callback (see
PrivacySettings.handleExportKeys) and pass only the already-derived keys forward
to ExportPrivacyKeys. Remove usage of pin/useParams from ExportPrivacyKeys and
update ExportPrivacyKeys to read spendXpub and scanXpriv from params (or receive
them via navigation state that you already use), and change the PinScreen
callback (the code that calls navigation to ExportPrivacyKeys in
PrivacySettings.handleExportKeys) to perform key derivation there and forward
only spendXpub and scanXpriv. Ensure no code path keeps the raw PIN in route
params or component state.
In `@src/screens/PrivacySettings.js`:
- Around line 52-60: PinScreen remains on the stack and retains the PIN in cb’s
closure. Update handleExportKeys so cb does not push another screen: replace
PinScreen with ExportPrivacyKeys using navigation.replace (not
navigation.navigate) so back returns to PrivacySettings and the closure is
discarded; do not pass { pin }. Prefer deriving the keys inside cb (or via a
saga) and forward only { spendXpub, scanXpriv } to ExportPrivacyKeys. If replace
isn’t available for this navigator, first pop PinScreen, then navigate to
ExportPrivacyKeys.
---
Duplicate comments:
In `@ios/HathorMobile.xcodeproj/project.pbxproj`:
- Line 489: The MARKETING_VERSION entries in the Xcode project (symbol
MARKETING_VERSION in project.pbxproj) are set to 0.38.99999 but must match
package.json (currently 0.0.1-shielded) to satisfy the CI check_version job;
update both MARKETING_VERSION occurrences to the exact version string from
package.json (or update package.json if intended to change the release version),
verify both occurrences (the two MARKETING_VERSION entries) are identical, and
re-run the check_version CI job before merging.
---
Nitpick comments:
In `@patches/`@hathor+wallet-lib+0.0.9-shielded.patch:
- Around line 26-34: The current conditional in the if block that checks
e.message uses a broad substring match e.message.indexOf("can't convert") which
can mask unrelated SyntaxErrors; tighten this by matching the specific JSC
message (e.g., "can't convert string to bigint") instead of the fragment and use
String.prototype.includes or an exact equality/regex to match that full phrase,
and replace the indexOf('Failed to parse String to BigInt') similarly with
includes; update the conditional around the SyntaxError check (the block
referencing e, context?.source and the BigInt message comparisons) to use these
narrower comparisons.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60c70dc1-e9f7-4906-b655-cfe5df0292f0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
ios/HathorMobile.xcodeproj/project.pbxprojpackage.jsonpatches/@hathor+wallet-lib+0.0.9-shielded.patchsrc/App.jssrc/screens/ExportPrivacyKeys.jssrc/screens/PrivacySettings.js
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/App.js
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "set -e\n\nexport NODE_BINARY=node\n../node_modules/react-native/scripts/react-native-xcode.sh\n"; | ||
| shellScript = "set -e\n\nexport NODE_BINARY=/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node\n../node_modules/react-native/scripts/react-native-xcode.sh\n"; |
There was a problem hiding this comment.
Hardcoded Nix store path for NODE_BINARY will break non-Nix builds.
export NODE_BINARY=/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node
This pins the React Native bundle phase to one specific Nix store derivation. Any contributor (or CI runner) without that exact path, or after a Nix shell GC/rebuild that changes the hash, will get a build failure. Either revert to the default react-native-xcode.sh behavior (which finds node via $PATH) or read the path from an env-/user-defined setting outside the committed .pbxproj.
🔧 Suggested revert
-shellScript = "set -e\n\nexport NODE_BINARY=/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node\n../node_modules/react-native/scripts/react-native-xcode.sh\n";
+shellScript = "set -e\n\nWITH_ENVIRONMENT=\"../node_modules/react-native/scripts/xcode/with-environment.sh\"\nREACT_NATIVE_XCODE=\"../node_modules/react-native/scripts/react-native-xcode.sh\"\n\n/bin/sh -c \"$WITH_ENVIRONMENT $REACT_NATIVE_XCODE\"\n";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| shellScript = "set -e\n\nexport NODE_BINARY=/nix/store/7mb2v7kigiwxjza6k0vp4zg9354w1k1q-devshell-dir/bin/node\n../node_modules/react-native/scripts/react-native-xcode.sh\n"; | |
| shellScript = "set -e\n\nWITH_ENVIRONMENT=\"../node_modules/react-native/scripts/xcode/with-environment.sh\"\nREACT_NATIVE_XCODE=\"../node_modules/react-native/scripts/react-native-xcode.sh\"\n\n/bin/sh -c \"$WITH_ENVIRONMENT $REACT_NATIVE_XCODE\"\n"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/HathorMobile.xcodeproj/project.pbxproj` at line 292, The pbxproj contains
a hardcoded Nix store path assigned to NODE_BINARY in the shellScript entry
which will break builds on machines without that exact store path; remove the
fixed export line (the export NODE_BINARY=... assignment) from the shellScript
that runs ../node_modules/react-native/scripts/react-native-xcode.sh so the
script can resolve node from PATH, or alternatively make it read from an
external env/user setting by exporting NODE_BINARY only if not already set (i.e.
respect an existing $NODE_BINARY) before invoking react-native-xcode.sh; update
the shellScript entry accordingly to stop committing a fixed Nix store path.
| CLANG_ENABLE_MODULES = YES; | ||
| CODE_SIGN_ENTITLEMENTS = HathorMobile/HathorMobile.entitlements; | ||
| CURRENT_PROJECT_VERSION = 1.0.0; | ||
| CURRENT_PROJECT_VERSION = 0.7.0; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the project's version-check script to confirm the expected integer format
fd -t f 'check_version' -x cat {}
fd -t f 'check_version*.sh' -x cat {}
rg -n 'CURRENT_PROJECT_VERSION|MARKETING_VERSION' scripts 2>/dev/null || trueRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 7456
🏁 Script executed:
#!/bin/bash
# Check the current version in package.json
echo "=== package.json version ==="
grep '"version":' package.json | head -1
# Check current CURRENT_PROJECT_VERSION values at lines 475 and 511
echo ""
echo "=== Line 475 context ==="
sed -n '470,480p' ios/HathorMobile.xcodeproj/project.pbxproj
echo ""
echo "=== Line 511 context ==="
sed -n '506,516p' ios/HathorMobile.xcodeproj/project.pbxproj
# Also check MARKETING_VERSION for context
echo ""
echo "=== All CURRENT_PROJECT_VERSION occurrences ==="
grep -n "CURRENT_PROJECT_VERSION" ios/HathorMobile.xcodeproj/project.pbxprojRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1285
CURRENT_PROJECT_VERSION = 0.7.0 format does not match the validation requirements and will fail CI.
The check_version script validates CURRENT_PROJECT_VERSION using version-specific patterns, not integers:
- For release versions: expects
1.0.Xformat - For RC versions (when package.json has a hyphen): expects
0.{RC_NUMBER}.Xformat
The current package version is 0.0.1-shielded. Since the RC part (shielded) contains no digits, the validation pattern becomes malformed (0..X), making the current 0.7.0 value invalid.
Change the RC version in package.json from 0.0.1-shielded to a properly formatted RC like 0.0.1-rc1, which would then require CURRENT_PROJECT_VERSION = 0.1.X format. Alternatively, if this should be a release build, use package.json version 1.0.0 and set CURRENT_PROJECT_VERSION = 1.0.X.
Also applies to: 511-511
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/HathorMobile.xcodeproj/project.pbxproj` at line 475, The
CURRENT_PROJECT_VERSION entry `CURRENT_PROJECT_VERSION = 0.7.0;` does not match
the project's version-validation patterns; update the package.json version
and/or the CURRENT_PROJECT_VERSION to a matching format: either change
package.json from `0.0.1-shielded` to a numeric RC like `0.0.1-rc1` (which will
require CURRENT_PROJECT_VERSION to follow `0.1.X`), or make this a release by
setting package.json to `1.0.0` and then set CURRENT_PROJECT_VERSION to the
release pattern `1.0.X`; ensure the two values align with the check_version
script rules so CI validation passes.
| + // [RN PATCH] Hermes and JSC do not pass the `context` arg to JSON | ||
| + // revivers (Node-22 / V8 11.5+ feature). Without it we cannot | ||
| + // recover the original numeric token, so we cannot detect | ||
| + // unsafe-range integers. Returning the parsed `value` as-is | ||
| + // matches the safe-range path of the modern reviver — the upstream | ||
| + // Zod schemas use `bigIntCoercibleSchema` which accepts numbers. | ||
| + if (context === undefined || context === null) { | ||
| + return value; | ||
| + } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find use sites of JSONBigInt / bigint utilities and any places that consume large numeric fields
rg -nP --type=js --type=ts -C2 '\bJSONBigInt\b|bigIntCoercibleSchema|fromUnits|toBigInt|BigInt\s*\(' -g '!node_modules/**' -g '!**/dist/**'
rg -nP --type=js --type=ts -C2 '\b(amount|value|balance|tokenAmount|fee)\b\s*:' srcRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 25947
🏁 Script executed:
# Search for USE_HERMES setting and project configuration
find . -name "project.pbxproj" -o -name "*.pbxproj" | head -5Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 123
🏁 Script executed:
# Look for JSON.parse calls and JSONBigInt usage patterns
rg -nP --type=js --type=ts 'JSON\.parse|JSONBigInt|reviver' -B2 -A2 -g '!node_modules/**' -g '!**/dist/**'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1563
🏁 Script executed:
# Search for network/API response handling
rg -nP --type=js --type=ts 'fetch|axios|request|response|\.json\(\)' src/ | head -30Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 2943
🏁 Script executed:
# Check if there's any documentation about numeric value ranges or constraints
find . -name "*.md" -type f | xargs grep -l "bigint\|BigInt\|safe.*integer\|2\^53" 2>/dev/null | head -10Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 766
🏁 Script executed:
# Check if USE_HERMES is actually enabled in the Xcode project file
grep -n "USE_HERMES" ./ios/HathorMobile.xcodeproj/project.pbxprojRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 132
🏁 Script executed:
# Look for bigIntUtils or JSONBigInt imports to see how it's being used
rg -nP 'import.*JSONBigInt|bigIntUtils|reviver' -g '!node_modules/**' src/Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 327
🏁 Script executed:
# Check the actual patch file to understand what it's patching
cat patches/@hathor+wallet-lib+0.0.9-shielded.patch | head -100Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 2132
🏁 Script executed:
# Check hathor-wallet-lib version to understand BigInt handling in upstream
rg -n "bigIntCoercibleSchema|BigInt" patches/@hathor+wallet-lib+0.0.9-shielded.patch | head -20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 860
🏁 Script executed:
# Check if there's any protocol documentation about numeric value ranges
find . -name "*.md" -path "./!(node_modules)" -type f | xargs grep -l "amount\|value\|satoshi\|range" 2>/dev/null | head -5Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Verify all JSON parsing paths to see if JSONBigInt reviver is configured anywhere
rg -nP 'JSON\.parse.*reviver|JSON\.parse.*bigInt|JSONBigInt.*parse' -g '!node_modules/**' src/Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Search for places where hathorLib or wallet-lib is being used for network operations
rg -nP 'hathorLib\.|wallet-lib' src/ --type=js --type=ts -B2 -A2 | grep -E '(parse|response|fetch|transaction|balance|amount)' | head -40Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 3579
🏁 Script executed:
# Look for how transaction data or balance data is actually parsed from network
rg -nP 'getBalance|getTx|fetchTx|wallet\.' src/ --type=js --type=ts -A5 | head -50Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 2487
🏁 Script executed:
# Check for any configuration or constants that define max values or ranges
rg -nP 'MAX_SAFE|MIN_SAFE|MAX_.*INTEGER|2\^53|9007199254740991' -g '!node_modules/**' src/Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Look at the shielded crypto provider to see if values are actually large
cat -n src/shieldedCryptoProvider.js | head -110Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 5336
🏁 Script executed:
# Check if wallet-lib's JSONBigInt.parse or reviver is actually called in the app
rg -nP 'JSONBigInt\.parse|parse.*reviver|reviver.*parse' -g '!node_modules/**' src/Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Look for what hathorLib is and how transactions/balances are fetched
rg -nP 'import.*hathorLib|wallet.getBalance|wallet.getTx' -g '!node_modules/**' src/ | head -20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 1813
🏁 Script executed:
# Check the actual data structures received from network in real usage
rg -nP 'const\s+\w+\s*=\s*await\s+(wallet|hathorLib)' -g '!node_modules/**' src/ --type=js -A3 | head -40Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 2334
Silent precision loss for unsafe-range BigInts on Hermes/JSC — confirmed as unavoidable but mitigated.
When context is absent (Hermes/JSC limitation), the reviver returns the parsed JS Number directly without detection of values outside Number.MIN_SAFE_INTEGER..Number.MAX_SAFE_INTEGER. This patch is a necessary workaround since Hermes does not pass the context parameter to JSON revivers, but it creates risk: any JSON number with magnitude > 2^53 will be silently truncated to the nearest representable double. Since USE_HERMES = true is configured for both Debug and Release in ios/HathorMobile.xcodeproj/project.pbxproj, this is now the default iOS behavior.
The patch comment notes that upstream @hathor/wallet-lib expects callers to rely on bigIntCoercibleSchema as a secondary validation layer. The app does use this schema in src/utils.js and src/components/Reown/NanoContract/BaseNanoContractRequest.js. However, confirm that all code paths receiving parsed numeric data from the network go through bigIntCoercibleSchema validation and verify that the wire protocol's numeric fields are never expected to exceed the safe-integer range. If on-the-wire values (token balances, amounts, timestamps, etc.) can exceed 2^53, callers need explicit documentation or a BigInt-aware parallel path; otherwise, rely on the secondary validation being comprehensively applied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@patches/`@hathor+wallet-lib+0.0.9-shielded.patch around lines 9 - 17, The
reviver fallback returns JS Numbers when `context` is undefined, causing silent
precision loss for unsafe-range integers on Hermes/JSC; locate the reviver
(references: `context`, `value`) and ensure every network-parsed numeric field
is validated through the BigInt-aware schema `bigIntCoercibleSchema` (or routed
to a BigInt-aware parsing path) before use — audit and update all call sites (at
minimum `src/utils.js` and
`src/components/Reown/NanoContract/BaseNanoContractRequest.js`) to run that
validation, add explicit runtime checks that error/log if a numeric value
exceeds Number.MAX_SAFE_INTEGER when `context` is missing, and add brief
documentation noting `USE_HERMES = true` implications so callers know to use the
BigInt-aware path.
| import { | ||
| ActivityIndicator, | ||
| Clipboard, | ||
| ScrollView, | ||
| StyleSheet, | ||
| Text, | ||
| TouchableOpacity, | ||
| View, | ||
| } from 'react-native'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm RN version + check for any existing clipboard usage / dependency.
fd -t f 'package.json$' --max-depth 2 --exec rg -nH '"react-native"|"@react-native-clipboard/clipboard"|"expo-clipboard"' {}
echo '--- existing Clipboard imports across the repo ---'
rg -nP "from\s+['\"]react-native['\"]" -g '*.{js,jsx,ts,tsx}' -A0 -B0 | rg -n "Clipboard"
rg -nP "from\s+['\"]@react-native-clipboard/clipboard['\"]|from\s+['\"]expo-clipboard['\"]" -g '*.{js,jsx,ts,tsx}'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 713
🏁 Script executed:
head -50 src/screens/ExportPrivacyKeys.js | tail -10Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 264
🏁 Script executed:
sed -n '1,20p' src/screens/ExportPrivacyKeys.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 517
Critical: Clipboard was removed from react-native core in 0.77 — Copy will crash at runtime.
React Native 0.77.2 (pinned in this repo's package.json) removed Clipboard from core. The import on lines 9–17 resolves to undefined, and tapping the Copy button on line 44 throws TypeError: Cannot read properties of undefined (reading 'setString'). The user can still read the revealed key on screen, but the documented "typical user flow is to copy the key into an audit tool" is broken.
This issue affects at least 5 files in the codebase. Replace the react-native import with @react-native-clipboard/clipboard (or expo-clipboard, depending on what's already standard in this repo):
🐛 Proposed fix
-import {
- ActivityIndicator,
- Clipboard,
- ScrollView,
- StyleSheet,
- Text,
- TouchableOpacity,
- View,
-} from 'react-native';
+import {
+ ActivityIndicator,
+ ScrollView,
+ StyleSheet,
+ Text,
+ TouchableOpacity,
+ View,
+} from 'react-native';
+import Clipboard from '@react-native-clipboard/clipboard';Also affects: src/components/CopyClipboard.js, src/components/AuditUnblindingRows.js, src/components/Reown/TokenInfoModal.js, src/components/Reown/SendTransactionRequest.js.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { | |
| ActivityIndicator, | |
| Clipboard, | |
| ScrollView, | |
| StyleSheet, | |
| Text, | |
| TouchableOpacity, | |
| View, | |
| } from 'react-native'; | |
| import { | |
| ActivityIndicator, | |
| ScrollView, | |
| StyleSheet, | |
| Text, | |
| TouchableOpacity, | |
| View, | |
| } from 'react-native'; | |
| import Clipboard from '@react-native-clipboard/clipboard'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/ExportPrivacyKeys.js` around lines 9 - 17, The Clipboard symbol
is imported from react-native core (e.g., in ExportPrivacyKeys.js and components
CopyClipboard.js, AuditUnblindingRows.js, Reown/TokenInfoModal.js,
Reown/SendTransactionRequest.js) but is removed in RN 0.77; replace those
imports by importing Clipboard from '@react-native-clipboard/clipboard' and keep
using the existing API (e.g., Clipboard.setString(text)); update any
default/named import forms to match the new package, run a quick grep for "from
'react-native' ... Clipboard" to find all occurrences and update them, and run
the app/linter to ensure no remaining undefined Clipboard usages remain.
| const onCopy = () => { | ||
| Clipboard.setString(value); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 1500); | ||
| }; |
There was a problem hiding this comment.
Auto-clear sensitive key material from the system clipboard.
The scan xpriv decrypts every shielded amount/token this wallet receives. Once Clipboard.setString(value) runs, that string stays in the OS clipboard indefinitely (until something else is copied) — on Android prior to API 33 any foreground app can read it, and on iOS Universal Clipboard syncs it to nearby devices. The 1.5 s Copied state on screen is purely UI feedback and doesn't bound clipboard residence.
Schedule a clipboard wipe (e.g., 60 s) after copying, and document the trade-off in the warning card.
🛡️ Proposed fix
const onCopy = () => {
Clipboard.setString(value);
setCopied(true);
setTimeout(() => setCopied(false), 1500);
+ // Wipe the clipboard if it still holds this key after 60s, so the
+ // xpriv doesn't linger after the user finishes pasting into the
+ // audit tool. Best-effort: compare-and-clear avoids stomping on
+ // anything the user copied in the meantime.
+ setTimeout(async () => {
+ try {
+ const current = await Clipboard.getString();
+ if (current === value) Clipboard.setString('');
+ } catch (_) { /* swallow — clipboard may be unreadable */ }
+ }, 60_000);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onCopy = () => { | |
| Clipboard.setString(value); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }; | |
| const onCopy = () => { | |
| Clipboard.setString(value); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| // Wipe the clipboard if it still holds this key after 60s, so the | |
| // xpriv doesn't linger after the user finishes pasting into the | |
| // audit tool. Best-effort: compare-and-clear avoids stomping on | |
| // anything the user copied in the meantime. | |
| setTimeout(async () => { | |
| try { | |
| const current = await Clipboard.getString(); | |
| if (current === value) Clipboard.setString(''); | |
| } catch (_) { /* swallow — clipboard may be unreadable */ } | |
| }, 60_000); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/ExportPrivacyKeys.js` around lines 43 - 47, The clipboard copy
handler onCopy currently calls Clipboard.setString(value) and only sets a UI
"copied" flag for 1.5s, leaving sensitive xpriv in the OS clipboard
indefinitely; update onCopy to schedule an automated clipboard wipe (e.g., call
Clipboard.setString('') or an OS-appropriate clear API) after a short window
such as 60s using a timeout and ensure that any pending wipe is cleared if the
component unmounts; also update the warning card text in ExportPrivacyKeys to
document this trade-off and the chosen wipe delay so users are informed.
| const ExportPrivacyKeys = () => { | ||
| const navigation = useNavigation(); | ||
| const wallet = useSelector((state) => state.wallet); | ||
| // The PIN is passed in by the PinScreen callback that brought us | ||
| // here. We read it once on mount, derive both keys, and let it fall | ||
| // out of memory when this screen unmounts (route params get | ||
| // garbage-collected with the popped frame). | ||
| const params = useParams(); | ||
| const pin = params?.pin ?? null; | ||
|
|
||
| const [keys, setKeys] = useState({ spendXpub: null, scanXpriv: null }); | ||
| const [error, setError] = useState(null); | ||
| const [loading, setLoading] = useState(true); | ||
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
| const derive = async () => { | ||
| try { | ||
| if (!wallet?.storage) { | ||
| throw new Error(t`Wallet not ready`); | ||
| } | ||
| if (!pin) { | ||
| throw new Error(t`Missing PIN; please re-enter from the previous screen.`); | ||
| } | ||
| const [spendXpub, scanXpriv] = await Promise.all([ | ||
| wallet.storage.getSpendXPubKey(), | ||
| wallet.storage.getScanXPrivKey(pin), | ||
| ]); | ||
| if (cancelled) return; | ||
| if (!spendXpub) { | ||
| throw new Error( | ||
| t`This wallet has no shielded keys. They are only present on wallets created after the shielded outputs feature was enabled.` | ||
| ); | ||
| } | ||
| setKeys({ spendXpub, scanXpriv }); | ||
| } catch (e) { | ||
| if (cancelled) return; | ||
| setError(e instanceof Error ? e.message : String(e)); | ||
| } finally { | ||
| if (!cancelled) setLoading(false); | ||
| } | ||
| }; | ||
| derive(); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [wallet, pin]); | ||
|
|
||
| return ( | ||
| <View style={styles.screen}> | ||
| <HathorHeader | ||
| title={t`EXPORT PRIVACY KEYS`} | ||
| onBackPress={() => navigation.goBack()} | ||
| /> | ||
| <ScrollView | ||
| contentContainerStyle={styles.scrollContent} | ||
| keyboardShouldPersistTaps='handled' | ||
| > | ||
| <View style={styles.warningCard}> | ||
| <Text style={styles.warningTitle}>{t`Read this before sharing`}</Text> | ||
| <Text style={styles.warningBody}> | ||
| {t`These two keys give an auditor read-only access to every shielded output your wallet receives — they can identify every incoming shielded transaction and decrypt the amount and token of each one.`} | ||
| </Text> | ||
| <Text style={styles.warningBody}> | ||
| {t`They do NOT allow spending. Whoever holds them cannot move your funds, sign transactions on your behalf, or access transparent (non-shielded) balances.`} | ||
| </Text> | ||
| <Text style={styles.warningBody}> | ||
| {t`Treat them like any other private wallet data. Share them only with auditors you trust, and over a secure channel.`} | ||
| </Text> | ||
| </View> | ||
|
|
||
| {loading && ( | ||
| <View style={styles.loadingBlock}> | ||
| <ActivityIndicator size='small' color={COLORS.primary} /> | ||
| <Text style={styles.loadingText}>{t`Deriving keys…`}</Text> | ||
| </View> | ||
| )} | ||
|
|
||
| {error && ( | ||
| <View style={styles.errorCard}> | ||
| <Text style={styles.errorText}>{error}</Text> | ||
| </View> | ||
| )} | ||
|
|
||
| {!loading && !error && keys.spendXpub && ( | ||
| <KeyCard | ||
| label={t`Spend public key (xpub)`} | ||
| description={ | ||
| t`Path m/44'/280'/2'/0. Lets the auditor derive every receiving spend-side address and find your incoming shielded outputs on-chain.` | ||
| } | ||
| value={keys.spendXpub} | ||
| /> | ||
| )} | ||
|
|
||
| {!loading && !error && keys.scanXpriv && ( | ||
| <KeyCard | ||
| label={t`Scan private key (xpriv)`} | ||
| description={ | ||
| t`Path m/44'/280'/1'/0. Lets the auditor decrypt the value and token of every shielded output your wallet receives. Used only for ECDH; never for signing.` | ||
| } | ||
| value={keys.scanXpriv} | ||
| /> | ||
| )} | ||
| </ScrollView> | ||
| </View> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether any screen-capture protection is already wired up in the app.
rg -nP 'FLAG_SECURE|setFlags|screen-capture|screenshot-prevent|react-native-screen-capture-secure|enableSecureView|preventScreenshot' -g '!**/node_modules/**'
fd -t f 'package.json$' --max-depth 2 --exec rg -nH 'screen-capture|screenshot-prevent|prevent-screenshot' {}Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Find ExportPrivacyKeys.js and verify it exists
fd -type f 'ExportPrivacyKeys.js'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 249
🏁 Script executed:
# Check the root package.json and lock files for screenshot protection libraries
cat package.json | head -100Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 3777
🏁 Script executed:
# Search more thoroughly for any screen protection implementations with variations
rg -i 'secure|FLAG_SECURE|screenshot|screen.?capture|blur.*active|resign' -t ts -t tsx -t js -t jsx --max-count 20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 106
🏁 Script executed:
# Check for PinScreen implementation
fd -type f 'PinScreen.js' -o -name 'PinScreen.tsx'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 249
🏁 Script executed:
# Find the files using correct syntax
fd -t f 'ExportPrivacyKeys.js'
fd -t f 'PinScreen.js'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 167
🏁 Script executed:
# Search for any screen/security related code without type flags
rg 'FLAG_SECURE|setFlags|screen.*secure|blur.*resign|capturedDidChange|FLAG_SECURE' --max-count 20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 60
🏁 Script executed:
# Check if there are any native module configurations in the repo
find . -name 'android' -type d | head -5
find . -name 'ios' -type d | head -5Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 775
🏁 Script executed:
# Verify the package.json more thoroughly for any missing dependencies
grep -i 'screen\|screenshot\|secure\|FLAG' package.jsonRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 116
🏁 Script executed:
# Read ExportPrivacyKeys.js to verify content
cat -n src/screens/ExportPrivacyKeys.js | head -220Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 9192
🏁 Script executed:
# Read PinScreen.js to check for any existing protection
cat -n src/screens/PinScreen.js | head -150Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 6121
🏁 Script executed:
# Check native Android code for any FLAG_SECURE implementation
find ./android -name '*.java' -o -name '*.kt' | head -10Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 273
🏁 Script executed:
# Check native iOS code for any secure window handling
find ./ios -name '*.swift' -o -name '*.m' | head -10Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 251
🏁 Script executed:
cat -n ./android/app/src/main/java/com/hathormobile/MainActivity.ktRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1468
🏁 Script executed:
cat -n ./ios/HathorMobile/AppDelegate.swiftRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 1637
No screenshot / screen-recording protection while highly-sensitive audit keys are on screen.
This screen renders the scan xpriv (and a revealable spend xpub). Once revealed on-screen, the sensitive keys can appear in screenshots, screen recordings, or — on Android — the recents-thumbnail snapshot. For a wallet-grade audit-keys export, standard hardening is required:
- Android: set
FLAG_SECUREon the activity window for the lifetime of this screen. - iOS: blur on
applicationWillResignActive(recents snapshot) and optionally listen forUIScreen.capturedDidChangeNotificationto warn / blur during recording.
Libraries like react-native-screen-capture-secure or react-native-screenshot-prevent wrap both. Apply on mount of ExportPrivacyKeys and revert on unmount; apply the same protection to PinScreen.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/ExportPrivacyKeys.js` around lines 106 - 211, ExportPrivacyKeys
currently displays highly sensitive keys without preventing
screenshots/recents/recordings; fix by enabling screen-capture protection on
mount and disabling on unmount (use useEffect in ExportPrivacyKeys to call a
screen-security library such as react-native-screen-capture-secure or
react-native-screenshot-prevent, which sets Android FLAG_SECURE and applies iOS
blurring/capture listeners), and apply the same mount/unmount protection in
PinScreen so the PIN entry and the path that navigates to ExportPrivacyKeys are
also protected; ensure you call the library's enable/disable (or equivalent)
APIs, and attach any iOS capturedDidChange/applicationWillResignActive handlers
for blur/warning while the screen is active.
| const navigation = useNavigation(); | ||
| const wallet = useSelector((state) => state.wallet); | ||
| // The PIN is passed in by the PinScreen callback that brought us | ||
| // here. We read it once on mount, derive both keys, and let it fall | ||
| // out of memory when this screen unmounts (route params get | ||
| // garbage-collected with the popped frame). | ||
| const params = useParams(); | ||
| const pin = params?.pin ?? null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify nav-state persistence is NOT enabled (otherwise the PIN can hit disk).
rg -nP 'persistNavigationState|onStateChange|initialState\s*=' -g '!**/node_modules/**' -g '*.{js,jsx,ts,tsx}'
# Locate PinScreen to understand what the callback runs with.
fd -t f -iI 'PinScreen' -e js -e jsx -e ts -e tsxRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 184
🏁 Script executed:
# Search for navigation setup and persistence configuration
fd -t f -name 'App.js' -o -name 'App.tsx' -o -name 'Navigation.js' -o -name 'Navigation.tsx' | head -20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 309
🏁 Script executed:
# Get more context on navigation configuration
find . -type f \( -name '*.js' -o -name '*.jsx' -o -name '*.ts' -o -name '*.tsx' \) | xargs grep -l 'NavigationContainer\|createNativeStackNavigator' | head -10Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 118
🏁 Script executed:
# Read PinScreen.js to understand the callback mechanism
cat -n src/screens/PinScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 16860
🏁 Script executed:
# Read ExportPrivacyKeys.js full context (around lines 107-114 and 148-152)
cat -n src/screens/ExportPrivacyKeys.js | head -160Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 6489
🏁 Script executed:
# Search for PrivacySettings.handleExportKeys
rg -n 'handleExportKeys' -A 10 src/screens/PrivacySettings.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 776
🏁 Script executed:
cat -n src/App.js | head -150Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 8434
🏁 Script executed:
# Check if route params are cleared or reset anywhere
rg -n 'params|route.params' src/ | head -40Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 3584
🏁 Script executed:
# Continue reading App.js to find NavigationContainer setup
cat -n src/App.js | sed -n '200,400p'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 7875
🏁 Script executed:
# Search for NavigationContainer configuration and any persistence setup
rg -n 'NavigationContainer' src/App.js -A 20Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 3043
🏁 Script executed:
# Double-check the exact flow: verify goBack is called before callback in PinScreen
sed -n '170,180p' src/screens/PinScreen.jsRepository: HathorNetwork/hathor-wallet-mobile
Length of output: 423
🏁 Script executed:
# Check if there's any cleanup of route params after ExportPrivacyKeys uses them
cat -n src/screens/ExportPrivacyKeys.js | sed -n '145,160p'Repository: HathorNetwork/hathor-wallet-mobile
Length of output: 512
PIN travelling through React Navigation route params creates unnecessary exposure.
The header comment claims route params are safely scoped to this screen's frame, but route params live in the navigator's state tree. While persistNavigationState is not configured in App.js (mitigating disk-write risk), two concerns remain:
- Flipper / React Native debuggers can inspect full navigation state including params.
- Route params persist in nav state while
ExportPrivacyKeysis mounted, even if the PIN was generated in a dismissed screen.
Safer pattern: derive both keys inside the PinScreen callback (see PrivacySettings.handleExportKeys line 53-56) and pass already-derived { spendXpub, scanXpriv } forward — the PIN never leaves PinScreen's callback scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/ExportPrivacyKeys.js` around lines 107 - 114, ExportPrivacyKeys
currently reads the raw PIN from route params via useParams (pin) which exposes
sensitive data; instead derive spendXpub and scanXpriv inside the PinScreen
callback (see PrivacySettings.handleExportKeys) and pass only the
already-derived keys forward to ExportPrivacyKeys. Remove usage of pin/useParams
from ExportPrivacyKeys and update ExportPrivacyKeys to read spendXpub and
scanXpriv from params (or receive them via navigation state that you already
use), and change the PinScreen callback (the code that calls navigation to
ExportPrivacyKeys in PrivacySettings.handleExportKeys) to perform key derivation
there and forward only spendXpub and scanXpriv. Ensure no code path keeps the
raw PIN in route params or component state.
| const handleExportKeys = () => { | ||
| navigation.navigate('PinScreen', { | ||
| cb: (pin) => { | ||
| navigation.navigate('ExportPrivacyKeys', { pin }); | ||
| }, | ||
| canCancel: true, | ||
| screenText: t`Enter your 6-digit PIN to export the audit keys`, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
PinScreen stays on the back stack — back button returns user to PIN entry, and the PIN lives in the callback closure longer than necessary.
navigation.navigate('PinScreen', { cb }) pushes PinScreen; then cb calls navigation.navigate('ExportPrivacyKeys', { pin }), which pushes the export screen on top of PinScreen. Two problems:
- UX: From
ExportPrivacyKeys, pressing the system back / header back lands onPinScreen(still showing the PIN entry UI), not onPrivacySettings. The headeronBackPress={() => navigation.goBack()}inExportPrivacyKeyshas the same effect. - Security/lifecycle: The PIN remains captured in
cb's closure for as long as PinScreen is in the stack, in addition to being inExportPrivacyKeys's route params. The "garbage-collected with the popped frame" comment inExportPrivacyKeys.jsonly fully holds once PinScreen is also popped.
Fix by popping PinScreen before pushing ExportPrivacyKeys — or, better, derive the keys inside the cb (or via a saga) and forward { spendXpub, scanXpriv } so the PIN never leaves PinScreen.
♻️ Minimal fix — replace PinScreen instead of stacking on top
const handleExportKeys = () => {
navigation.navigate('PinScreen', {
cb: (pin) => {
- navigation.navigate('ExportPrivacyKeys', { pin });
+ // Pop PinScreen first so the back button returns to
+ // PrivacySettings, and PinScreen's closure (which still
+ // holds `pin`) is released as soon as the frame unmounts.
+ navigation.replace('ExportPrivacyKeys', { pin });
},
canCancel: true,
screenText: t`Enter your 6-digit PIN to export the audit keys`,
});
};Note: navigation.replace availability depends on the navigator type (stack supports it; tab/drawer don't). Verify PinScreen is mounted in a stack navigator before applying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/PrivacySettings.js` around lines 52 - 60, PinScreen remains on
the stack and retains the PIN in cb’s closure. Update handleExportKeys so cb
does not push another screen: replace PinScreen with ExportPrivacyKeys using
navigation.replace (not navigation.navigate) so back returns to PrivacySettings
and the closure is discarded; do not pass { pin }. Prefer deriving the keys
inside cb (or via a saga) and forward only { spendXpub, scanXpriv } to
ExportPrivacyKeys. If replace isn’t available for this navigator, first pop
PinScreen, then navigate to ExportPrivacyKeys.
Skips over the 0.0.10-shielded release. 0.0.11-shielded carries the sparse-decode parent-lookup fix (12 callsites centralized through the new findSpentOutput helper) — resolves the symptom where a shielded-to-shielded send displayed change as a positive per-tx delta and the fullnode rejected signed txs with "Failed to verify if elements are equal". The patch file (lib/utils/bigint.js Hermes/RN bigInt-reviver fix) applies cleanly to 0.0.11 — wallet-lib's bigint.js was untouched across the 0.0.9 → 0.0.11 window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Picks up the provider tokenUid hex-conversion fix. Before this bump, every FullShielded receive through the Node provider threw "Invalid token id" inside processNewTx, dropping the WS-delivered tx and leaving the mobile wallet's balance out of sync until a full reload. The Hermes/JSC bigInt-reviver patch (`lib/utils/bigint.js`) applies cleanly to 0.0.12 — wallet-lib's bigint.js wasn't touched between 0.0.11 and 0.0.12. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`AuditUnblindingRows` originally shipped its own `buildEnvelope` / `base64url` / `encodePayload` triple because wallet-lib had no canonical encoder at the time. wallet-lib 0.0.13-shielded added `encodeShieldedUnblindingPayload` as the single source of truth across mobile, headless, and the audit page; the previous bump commit just brought us past that floor. Drop the local copy and call the lib export directly. Wire output is byte-identical (same envelope schema, same RFC 4648 §5 base64url) so existing explorer parsers keep working unchanged.
Mobile's shielded crypto provider is rewritten to extend
`AbstractShieldedProvider` from the newly-published
`@hathor/ct-crypto-provider`. The native bridge gains a dedicated
`generateRandomBlindingFactor` UniFFI export so the old dummy-call
workaround is gone, plus `deriveAssetTag` and `createCommitment`
needed by the abstract class's composed `openAmount/FullShieldedCommitment`.
BigInt ↔ Double precision bug fixed at the bridge boundary: u64 values
now marshal as decimal `String` between JS and Swift (was `Double`,
silently truncating above 2^53).
Changes:
- src/shieldedCryptoProvider.js: subclass AbstractShieldedProvider;
drop the dummy-call workaround; String-marshal u64 values
- ios/HathorCtCrypto/HathorCtCryptoModule.swift: replace `Double` with
`NSString` for u64 args; add generateRandomBlindingFactor,
deriveAssetTag, createCommitment methods
- ios/HathorCtCrypto/HathorCtCryptoModule.m: matching RCT_EXTERN_METHOD
declarations
- patches/: rename `@hathor+wallet-lib+0.0.15-shielded.patch` →
`@hathor+wallet-lib+0.0.16-shielded.patch` so patch-package tracks
the new wallet-lib version. Patch content (Hermes/JSC `bigIntReviver`
fix targeting `lib/utils/bigint.js`) unchanged — the patched file
didn't move between versions.
- package.json: add @hathor/ct-crypto-provider@0.0.1-shielded;
bump @hathor/wallet-lib from 0.0.15-shielded to 0.0.16-shielded
Native rebuild required:
1. cd ../hathor-ct-crypto && cargo run --features uniffi-cli -p hathor-ct-crypto-node --bin uniffi-bindgen generate ...
2. Build the xcframework
3. Copy regenerated hathor_ct_crypto.swift + hathor_ct_cryptoFFI.h into ios/HathorCtCrypto/
4. Rebuild iOS app
Same recipe for Android (Kotlin UniFFI bindings + AAR).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Please go the the
Previewtab and select the appropriate Pull Request template:Summary by CodeRabbit
New Features
Chores