Open
Conversation
…and UI components
WalkthroughAdds end-to-end eCash (XEC) support: dependencies, Chronik client and API wrapper, address/UTXO utilities, network configs (main/test), keyring eCash private-key access and ecashSign background handler, send/verify UI and fee/UTXO helpers, activity handling, and tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SendUI as Send UI
participant VerifyUI as Verify UI
participant Background as BackgroundHandler
participant Keyring as KeyRing
participant Chronik as ChronikClient
participant Network as ECashNetwork
User->>SendUI: Enter tx details and click Send
SendUI->>VerifyUI: Open verify screen with txData
User->>VerifyUI: Confirm
VerifyUI->>Background: InternalMethods.ecashSign(params)
Background->>Network: resolve network & node info
Background->>Keyring: getPrivateKeyForECash(account)
Keyring-->>Background: privateKeyBuffer
Background->>Chronik: instantiate wallet, wallet.sync()
Chronik-->>Background: wallet state / UTXOs
Background->>Chronik: wallet.action().to(...).build().broadcast()
Chronik-->>Background: { success, txid } / errors
Background->>Keyring: zero-fill privateKey
Background-->>VerifyUI: { result: JSON.stringify({txid}) } or error
VerifyUI->>User: show success or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (7)
packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts (1)
119-126: Use network-configured dust instead of hardcoded546.Line 125 hardcodes dust threshold. This should come from network config to avoid drift between fee logic and network definitions.
♻️ Proposed refactor
interface FeeCalculationParams { @@ networkDecimals: number; + dustLimit?: number; fallbackByteSize?: number; } @@ const { @@ networkDecimals, + dustLimit = 546, fallbackByteSize = 219, } = params; @@ const result = calculateNativeXECFee( sendAmount, nonTokenUTXOs, selectedAsset.decimals, networkDecimals, + dustLimit, ); @@ const calculateNativeXECFee = ( sendAmount: string, sortedUTXOs: UTXO[], assetDecimals: number, networkDecimals: number, + dustLimit: number, ): FeeCalculationResult & { leftover?: string } => { @@ - if (toBN(leftover).lt(toBN(546)) && toBN(leftover).gte(toBN(0))) { + if (toBN(leftover).lt(toBN(dustLimit)) && toBN(leftover).gte(toBN(0))) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts` around lines 119 - 126, Replace the hardcoded dust value 546 in packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts with the network-configured dust threshold: obtain the dust value from the app's network config/provider (e.g. networkConfig.dust or network.getDustThreshold()) at the top of the module or inside the relevant function, convert it to a BN, and use that BN in the comparison with leftover (replace toBN(546) with toBN(networkDust)). Ensure you import or access the same network config object used elsewhere so the change keeps using the canonical dust setting and that the leftover comparisons still use BN types.packages/extension/src/ui/action/views/network-activity/index.vue (1)
251-257: Share the BTC/XEC terminal-update helper.This branch is identical to the Bitcoin path: same
BTCRawInfo, same status mapping, same persistence flow. Pulling both UTXO providers through one small helper would keep future fixes from drifting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/ui/action/views/network-activity/index.vue` around lines 251 - 257, The ecash branch duplicates the Bitcoin terminal-update logic (casting info to BTCRawInfo, checking isActivityUpdating, setting activity.status = ActivityStatus.success, assigning activity.rawInfo, then calling updateActivitySync(...).then(() => updateVisibleActivity(...))); extract this sequence into a shared helper (e.g., updateUtxoActivityTerminal or applyBtcXecTerminalUpdate) and call it from both the Bitcoin and ProviderName.ecash branches; ensure the helper accepts the activity, info (typed as BTCRawInfo), and the isActivityUpdating flag and performs the same status mapping and persistence via updateActivitySync and updateVisibleActivity so future fixes stay in one place.packages/extension/src/libs/background/internal/ecash-sign.test.ts (1)
108-124: Missing test for invalid destination address.The
ecashSignhandler validates the destination address format (lines 41-43 inecash-sign.ts), but there's no test case for this validation path.Proposed test case
+ it('should return error for invalid destination address', async () => { + const keyring = createKeyring(); + const result = await ecashSign( + keyring, + makeMessage([ + { + toAddress: 'invalid_address_format', + amount: '1000', + account: baseAccount, + networkName: NetworkNames.ECash, + }, + ]), + ); + + expect(result.error).toBeDefined(); + expect(result.error!.message).toContain('invalid destination address'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/libs/background/internal/ecash-sign.test.ts` around lines 108 - 124, Add a new unit test in ecash-sign.test.ts that exercises the destination address validation in the ecashSign handler: create a keyring with createKeyring(), call ecashSign with a message from makeMessage that includes a malformed toAddress (e.g. "invalid-address") for NetworkNames.ECash, and assert that result.error is defined and its message contains the expected invalid-address/destination validation text (the same validation path in ecash-sign.ts used by ecashSign). Match the structure of the existing "should return error when toAddress is missing" test (use async/await and the same helpers) so the new test covers the address-format validation branch.packages/extension/src/providers/ecash/libs/activity-handlers.ts (2)
21-21: Avoidas anytype assertion.Accessing
cashAddrPrefixvia(network as any)bypasses type safety. Consider extending the type definition or using a type guard.Proposed fix
- const cashAddrPrefix = (network as any).cashAddrPrefix ?? 'ecash'; + const cashAddrPrefix = + 'cashAddrPrefix' in network + ? (network as { cashAddrPrefix?: string }).cashAddrPrefix ?? 'ecash' + : 'ecash';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts` at line 21, The code uses a bypassing cast "(network as any).cashAddrPrefix"; replace this with a safe typed access by either extending the network type/interface to include cashAddrPrefix (e.g., declare an interface with cashAddrPrefix and use that type for network) or use a type guard before reading cashAddrPrefix (e.g., check "cashAddrPrefix" in network and narrow the type), then set const cashAddrPrefix = network.cashAddrPrefix ?? 'ecash' (referencing the network variable and the cashAddrPrefix identifier).
95-103: Potential precision loss when converting bigint sats to Number.
extractSatslikely returns abigint, andNumber()conversion can lose precision for values exceedingNumber.MAX_SAFE_INTEGER(~9 quadrillion satoshis). While unlikely for typical transactions, extremely large UTXOs could produce incorrect values.Consider keeping the values as strings or using a library like
bignumber.jsfor theBTCRawInfoif precision is critical.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts` around lines 95 - 103, The inputs/outputs mapping is converting sats via Number(extractSats(...)) which can lose precision; change the value fields to preserve full precision by returning the sats as strings (e.g., extractSats(...).toString()) or a BigNumber instead of Number; update the consumers/types (BTCRawInfo) to accept string or BigNumber and adjust any downstream parsing logic accordingly; locate the mappings in activity-handlers.ts (inputs: tx.inputs.map(...) and outputs: tx.outputs.map(...)) and the extractSats function to implement the safe conversion.packages/extension/src/providers/ecash/networks/ecash-base.ts (1)
82-91: Reuse one initializedChronikAPIperECashNetwork.
this.api()constructs a new client and callschronikInfo()on every balance, UTXO, or history request. Caching the initialized promise avoids extra latency and unnecessary load throughout the send flow.♻️ Suggested refactor
constructor(options: ECashNetworkOptions) { - const api = async () => { - const chronikApi = new ChronikAPI( - options.node, - options.networkInfo, - options.decimals, - options.name, - ); - await chronikApi.init(); - return chronikApi as ChronikAPI; - }; + let apiPromise: Promise<ChronikAPI> | undefined; + const api = async () => { + apiPromise ??= (async () => { + const chronikApi = new ChronikAPI( + options.node, + options.networkInfo, + options.decimals, + options.name, + ); + await chronikApi.init(); + return chronikApi; + })(); + return apiPromise; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/networks/ecash-base.ts` around lines 82 - 91, The current api factory (const api = async () => { ... ChronikAPI; }) creates and init()s a new ChronikAPI on every call; change it to cache the initialized promise on the ECashNetwork instance (e.g., this.chronikApiPromise or this._apiPromise) so subsequent calls to this.api() return the same Promise<ChronikAPI> instead of creating a new client; update the api() accessor to check the cached promise, construct and init a new ChronikAPI only if the cached promise is undefined, and reuse that promise for all chronikInfo()/balance/UTXO/history requests to avoid repeated init and extra latency.packages/extension/src/providers/ecash/tests/utils.test.ts (1)
15-34: Add regression coverage for network prefixes and signer change behavior.This suite only exercises mainnet
ecash:/ prefixless addresses and single-address ownership. Given the new testnet support, it should reject the wrong prefix, and once the signer behavior is confirmed, it should also cover sent transactions that emit change to another wallet-owned address.Also applies to: 110-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/tests/utils.test.ts` around lines 15 - 34, Update the isValidECashAddress test suite to add cases that assert network-prefix handling: include tests that accept the testnet eCash prefix and reject addresses with the wrong prefix (i.e., ensure isValidECashAddress('ectest:...') is treated per testnet context and that mismatched prefixes return false), and add a regression test for sent-transaction change ownership that constructs a sent transaction with a change output that resolves to another wallet-owned address and asserts the signer/change-detection behavior (exercise the code path that identifies change outputs — e.g., the function that determines change ownership used by the outgoing transaction processor — to ensure it recognizes change to another wallet address).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension/src/libs/activity-state/wrap-activity-handler.ts`:
- Around line 12-17: The cache key for activity is being normalized to
getAddressWithoutPrefix(address) in wrap-activity-handler.ts (cacheAddress) but
writes via updateActivitySync() in
packages/extension/src/ui/action/views/network-activity/index.vue still use the
display-form address, causing eCash duplicate/stale records; fix by normalizing
the write path to use the same key (call getAddressWithoutPrefix before
persisting/updating activity in updateActivitySync) or alternatively stop
normalizing cacheAddress in wrap-activity-handler.ts so both read and write use
the identical address form; ensure the same symbol getAddressWithoutPrefix and
the cache key used by wrap-activity-handler.ts and updateActivitySync() are
consistent.
In `@packages/extension/src/libs/background/index.ts`:
- Line 55: The registration of ProviderName.ecash inside the `#tabProviders` map
can cause runtime crashes because the Providers registry and background provider
type declarations aren't wired for eCash; update the Providers map (the object
keyed by provider names used when instantiating new
this.#providers[_provider](...)), and any background provider type/interface
declarations to include the eCash provider implementation and constructor
signature, or alternatively add a defensive guard where tab provider instances
are created (check this.#providers[_provider] exists before calling new) to
prevent instantiation when the provider is missing; ensure references to
ProviderName.ecash, `#tabProviders`, Providers, and the new
this.#providers[_provider](...) call are updated consistently.
In `@packages/extension/src/libs/background/internal/ecash-sign.ts`:
- Around line 101-102: When `result.success` is true but `result.broadcasted` is
an empty array, explicitly handle that case instead of returning an empty txid;
update the logic in ecash-sign.ts where `const txid = result.broadcasted[0] ||
''` and the subsequent `return { result: JSON.stringify({ txid }) }` to check
`result.broadcasted` length first (e.g., if `result.success &&
(!result.broadcasted || result.broadcasted.length === 0)`) and return a clear
failure/partial response (such as an error field or `success: false` with a
descriptive message) so downstream consumers don’t receive an empty txid.
In `@packages/extension/src/providers/ecash/libs/api-chronik.ts`:
- Around line 65-75: getBalance derives a watch-only address with
getAddress(pubkey) which uses the mainnet default; update it to respect this
instance's configured prefix by passing the ChronikAPI's
networkInfo.cashAddrPrefix (or otherwise using a network-aware
address-derivation helper) when deriving the address before calling
WatchOnlyWallet.fromAddress(address, this.chronik), so the wallet queries the
correct chain; modify getBalance to use the networkInfo.cashAddrPrefix (from
this.networkInfo or this.chronik.networkInfo) when calling getAddress.
In `@packages/extension/src/providers/ecash/libs/utils.ts`:
- Around line 4-11: isValidECashAddress currently only checks parseability
(Address.parse) and later getAddressWithoutPrefix strips network info, allowing
wrong-network addresses to be accepted; update isValidECashAddress to accept a
network/context parameter (or the expected prefix) and after
Address.parse(address) verify the parsed address's network/prefix matches the
expected network (e.g., compare addr.network or addr.prefix to the provided
network/prefix) and return false if it doesn't match; also ensure any callers
that rely on getAddressWithoutPrefix are updated to validate network before
normalization.
- Around line 66-83: calculateTransactionValue and getTransactionAddresses
misclassify HD-derived change outputs because they compare outputs against a
single normalizedAddress; update these helpers to accept (and use) the full set
of owned addresses (e.g., pass an array like ownedAddresses) instead of a single
normalizedAddress, then change the filtering in calculateTransactionValue (and
the output-address logic in getTransactionAddresses) to consider an output as
internal if scriptToAddress(output.outputScript, cashAddrPrefix) exists in
ownedAddresses; alternatively, make the functions accept an explicit list of
recipient addresses and treat any output not in recipients but in ownedAddresses
as change—adjust signatures for calculateTransactionValue and
getTransactionAddresses, update their callers to supply the ownedAddresses (or
recipients), and keep use of extractSats and toBN unchanged.
In `@packages/extension/src/providers/ecash/networks/ecash-base.ts`:
- Around line 44-63: createECashNetworkOptions currently hardcodes
cashAddrPrefix:'ecash' and a mainnet-style blockExplorerAddr, causing testnet
builds that set only networkInfo.cashAddrPrefix to still show mainnet addresses;
update the defaults so cashAddrPrefix and blockExplorerAddr derive from the
provided networkInfo or from isTestNetwork rather than hard constants.
Concretely, use options.cashAddrPrefix ?? options.networkInfo?.cashAddrPrefix ??
(options.isTestNetwork ? '<test-prefix>' : 'ecash') for cashAddrPrefix, and when
blockExplorerAddr is not supplied, compose a default URL using the resolved
cashAddrPrefix (or choose a testnet explorer when isTestNetwork is true) instead
of the fixed 'https://explorer.e.cash/address/ecash:[[address]]'; adjust the
blockExplorerTX default similarly and ensure references to networkInfo remain
unchanged (update symbols: createECashNetworkOptions, cashAddrPrefix,
blockExplorerAddr, blockExplorerTX, networkInfo).
In `@packages/extension/src/providers/ecash/types/ecash-network.ts`:
- Around line 57-60: The catch block in the pubkey-to-address conversion
silently returns the original pubkey, which can leak invalid data; change the
handler in the function that converts a pubkey to a CashAddr so it does not
return the raw pubkey on failure—instead either rethrow the caught error or
return a clear sentinel (prefer null or an empty string) and log the error with
context; update the catch in the conversion routine (the function that currently
logs "Error converting pubkey to cashaddr:") to return null (or throw) so
callers can detect and handle the failure explicitly.
In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts`:
- Around line 53-68: After filtering accountUTXOs into nonTokenUTXOs, check
whether nonTokenUTXOs is empty and handle that case before calling
calculateNativeXECFee; if empty, return an error/empty-fee result or
short-circuit the spend path (e.g., indicate no spendable UTXOs) so you don’t
attempt fee estimation with zero inputs. Update the logic around nonTokenUTXOs
and the call to calculateNativeXECFee to perform this guard and surface a clear
outcome to the caller.
In `@packages/extension/src/providers/ecash/ui/libs/send-utils.ts`:
- Around line 54-55: Clamp the computed maxValue to zero when fee can exceed
utxoBalance: after computing maxValue = utxoBalance.sub(toBN(toBase(fee,
networkDecimals))) check if maxValue.isNeg() (or maxValue.lt(toBN(0))) and set
it to toBN(0) before calling fromBase(maxValue.toString(), assetDecimals) so
fromBase never receives a negative value; reference symbols: utxoBalance, toBN,
toBase, fee, networkDecimals, maxValue, fromBase, assetDecimals.
- Around line 14-18: The current reduction over nonTokenUTXOs uses Number(...)
which loses precision; change the reduction to operate entirely in BN/string
space by calling toBN on each utxo amount (utxo.sats || utxo.value || '0') and
using BN addition inside the reducer (or by accumulating a BN via
toBN(acc).add(...)) so the function returning toBN(...) no longer wraps a JS
number; update both occurrences referenced (the reduce over nonTokenUTXOs and
the similar spot at line 45) to use toBN and BN.add instead of Number coercion.
In
`@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue`:
- Around line 36-39: The network prop is given an unsafe default of () => ({})
but code calls network.displayAddress() and network.identicon(), causing runtime
errors; change the Prop definition for network (PropType<ECashNetwork>) to
either make it required (remove default and set required: true) or provide a
safe default object implementing stub methods displayAddress() and identicon()
that return harmless values, and ensure the component uses the Prop name
`network` (and methods network.displayAddress()/network.identicon()) accordingly
so callers must supply a real ECashNetwork or the stubs prevent crashes.
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 380-384: The current sendAction calls
recentlySentAddresses.addRecentlySentAddress(props.network, addressTo.value)
before the transaction is verified and signed, causing failed or cancelled sends
to be persisted; remove that pre-sign call from sendAction and instead invoke
recentlySentAddresses.addRecentlySentAddress(...) only in the success branch
after InternalMethods.ecashSign returns a txid (i.e., the success path in
verify-transaction between the existing success handling at around the
verify-transaction success block), so the address is added only when the
broadcast/signing completes successfully.
- Around line 52-56: The component never updates the local selectedFee reactive
value from the child send-fee-select, so downstream computations (currentGasFee,
assetMaxValue, and the verify payload) always use GasPriceTypes.REGULAR; wire
the selector to the component state by adding a two-way binding (v-model) or
listening to the child’s update event and assign the new value to selectedFee;
ensure selectedFee is the single source of truth used wherever currentGasFee,
assetMaxValue, or the verify payload are computed so changes from
send-fee-select propagate immediately (also apply the same fix for the other
occurrence referenced near line 167).
In
`@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue`:
- Around line 220-230: The failure path stores the activity under the raw sender
id (txData.fromAddress) instead of the normalized key used on success; update
the catch block where activityState.addActivities is called (and the similar
block around the other catch) to use the normalized sender key
(displayFromAddress.value) like the success branch, i.e., pass address:
displayFromAddress.value when adding txActivity so failed sends are saved to the
same bucket as successful ones.
- Around line 112-116: The component currently decodes route.query.txData and
reads route.query.id / KeyRing.getAccount() during setup which can throw; move
the Base64 JSON decode of route.query.txData and any use of
route.query.id/KeyRing.getAccount() into an onBeforeMount block and wrap them in
try/catch so failures set an explicit error state (e.g., a reactive error
string/flag) instead of throwing; update logic in selectedNetwork and txData to
be initialized lazily (empty/null) in setup and assigned inside onBeforeMount on
success, and ensure the UI uses the error state to display a recoverable message
rather than remaining in a loading state.
In `@packages/extension/src/ui/action/utils/filters.ts`:
- Around line 42-61: The `<` marker should be determined from the formatter's
rounding precision instead of the hard-coded 1e-7 check; compute the smallest
displayable positive value as 10^(-maximumFractionDigits) (using the
maximumFractionDigits you just set), format values with the existing
Intl.NumberFormat (symbols: maximumFractionDigits, minimumFractionDigits,
finalValue, formatted, formatter), and prepend `< ` only when amount is > 0 and
amount < smallestDisplayableValue (i.e., the value is non-zero but below what
the formatter can show). Ensure you use the same formatter options so the marker
aligns with the actual formatted output.
In `@packages/keyring/src/index.ts`:
- Around line 398-429: getPrivateKeyForECash currently assumes imported private
keys are hex and calls hexToBuffer(keypair.privateKey); to fix, ensure imported
keys are validated/normalized to hex at ingest rather than here: update
addKeyPair (or its helper `#setPrivateKey`) to detect common encodings (hex
string, WIF, Buffer/Uint8Array) and either convert them to canonical hex (use
bufferToHex for raw bytes, decode WIF for WIF) or reject invalid formats, then
store only hex strings (WalletType.privkey). Alternatively, if you prefer the
check at use-site, validate keypair.privateKey inside getPrivateKeyForECash and
throw a clear error when it is not valid hex before calling hexToBuffer;
reference functions: addKeyPair, `#setPrivateKey`, getPrivateKeyForECash,
hexToBuffer, WalletType.privkey.
---
Nitpick comments:
In `@packages/extension/src/libs/background/internal/ecash-sign.test.ts`:
- Around line 108-124: Add a new unit test in ecash-sign.test.ts that exercises
the destination address validation in the ecashSign handler: create a keyring
with createKeyring(), call ecashSign with a message from makeMessage that
includes a malformed toAddress (e.g. "invalid-address") for NetworkNames.ECash,
and assert that result.error is defined and its message contains the expected
invalid-address/destination validation text (the same validation path in
ecash-sign.ts used by ecashSign). Match the structure of the existing "should
return error when toAddress is missing" test (use async/await and the same
helpers) so the new test covers the address-format validation branch.
In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts`:
- Line 21: The code uses a bypassing cast "(network as any).cashAddrPrefix";
replace this with a safe typed access by either extending the network
type/interface to include cashAddrPrefix (e.g., declare an interface with
cashAddrPrefix and use that type for network) or use a type guard before reading
cashAddrPrefix (e.g., check "cashAddrPrefix" in network and narrow the type),
then set const cashAddrPrefix = network.cashAddrPrefix ?? 'ecash' (referencing
the network variable and the cashAddrPrefix identifier).
- Around line 95-103: The inputs/outputs mapping is converting sats via
Number(extractSats(...)) which can lose precision; change the value fields to
preserve full precision by returning the sats as strings (e.g.,
extractSats(...).toString()) or a BigNumber instead of Number; update the
consumers/types (BTCRawInfo) to accept string or BigNumber and adjust any
downstream parsing logic accordingly; locate the mappings in
activity-handlers.ts (inputs: tx.inputs.map(...) and outputs:
tx.outputs.map(...)) and the extractSats function to implement the safe
conversion.
In `@packages/extension/src/providers/ecash/networks/ecash-base.ts`:
- Around line 82-91: The current api factory (const api = async () => { ...
ChronikAPI; }) creates and init()s a new ChronikAPI on every call; change it to
cache the initialized promise on the ECashNetwork instance (e.g.,
this.chronikApiPromise or this._apiPromise) so subsequent calls to this.api()
return the same Promise<ChronikAPI> instead of creating a new client; update the
api() accessor to check the cached promise, construct and init a new ChronikAPI
only if the cached promise is undefined, and reuse that promise for all
chronikInfo()/balance/UTXO/history requests to avoid repeated init and extra
latency.
In `@packages/extension/src/providers/ecash/tests/utils.test.ts`:
- Around line 15-34: Update the isValidECashAddress test suite to add cases that
assert network-prefix handling: include tests that accept the testnet eCash
prefix and reject addresses with the wrong prefix (i.e., ensure
isValidECashAddress('ectest:...') is treated per testnet context and that
mismatched prefixes return false), and add a regression test for
sent-transaction change ownership that constructs a sent transaction with a
change output that resolves to another wallet-owned address and asserts the
signer/change-detection behavior (exercise the code path that identifies change
outputs — e.g., the function that determines change ownership used by the
outgoing transaction processor — to ensure it recognizes change to another
wallet address).
In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts`:
- Around line 119-126: Replace the hardcoded dust value 546 in
packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts with the
network-configured dust threshold: obtain the dust value from the app's network
config/provider (e.g. networkConfig.dust or network.getDustThreshold()) at the
top of the module or inside the relevant function, convert it to a BN, and use
that BN in the comparison with leftover (replace toBN(546) with
toBN(networkDust)). Ensure you import or access the same network config object
used elsewhere so the change keeps using the canonical dust setting and that the
leftover comparisons still use BN types.
In `@packages/extension/src/ui/action/views/network-activity/index.vue`:
- Around line 251-257: The ecash branch duplicates the Bitcoin terminal-update
logic (casting info to BTCRawInfo, checking isActivityUpdating, setting
activity.status = ActivityStatus.success, assigning activity.rawInfo, then
calling updateActivitySync(...).then(() => updateVisibleActivity(...))); extract
this sequence into a shared helper (e.g., updateUtxoActivityTerminal or
applyBtcXecTerminalUpdate) and call it from both the Bitcoin and
ProviderName.ecash branches; ensure the helper accepts the activity, info (typed
as BTCRawInfo), and the isActivityUpdating flag and performs the same status
mapping and persistence via updateActivitySync and updateVisibleActivity so
future fixes stay in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e5aecf91-b95b-4567-bfc5-b9e2820f8e9e
⛔ Files ignored due to path filters (2)
packages/extension/src/providers/ecash/networks/icons/ecash.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (38)
packages/extension/package.jsonpackages/extension/src/libs/activity-state/wrap-activity-handler.tspackages/extension/src/libs/background/index.tspackages/extension/src/libs/background/internal/ecash-sign.test.tspackages/extension/src/libs/background/internal/ecash-sign.tspackages/extension/src/libs/background/internal/index.tspackages/extension/src/libs/keyring/keyring.tspackages/extension/src/libs/utils/initialize-wallet.tspackages/extension/src/libs/utils/networks.tspackages/extension/src/providers/ecash/libs/activity-handlers.tspackages/extension/src/providers/ecash/libs/api-chronik.tspackages/extension/src/providers/ecash/libs/utils.tspackages/extension/src/providers/ecash/networks/ecash-base.tspackages/extension/src/providers/ecash/networks/ecash-testnet.tspackages/extension/src/providers/ecash/networks/index.tspackages/extension/src/providers/ecash/tests/ecash.address.derivation.test.tspackages/extension/src/providers/ecash/tests/utils.test.tspackages/extension/src/providers/ecash/types/ecash-chronik.tspackages/extension/src/providers/ecash/types/ecash-network.tspackages/extension/src/providers/ecash/types/ecash-token.tspackages/extension/src/providers/ecash/ui/libs/fee-calculator.tspackages/extension/src/providers/ecash/ui/libs/send-utils.tspackages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vuepackages/extension/src/providers/ecash/ui/send-transaction/components/send-alert.vuepackages/extension/src/providers/ecash/ui/send-transaction/index.vuepackages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vuepackages/extension/src/types/base-network.tspackages/extension/src/types/messenger.tspackages/extension/src/types/provider.tspackages/extension/src/ui/action/utils/filters.tspackages/extension/src/ui/action/views/deposit/index.vuepackages/extension/src/ui/action/views/network-activity/index.vuepackages/extension/src/ui/action/views/send-transaction/index.vuepackages/extension/src/ui/action/views/verify-transaction/index.vuepackages/keyring/src/index.tspackages/keyring/tests/generate.test.tspackages/types/src/index.tspackages/types/src/networks.ts
packages/extension/src/libs/activity-state/wrap-activity-handler.ts
Outdated
Show resolved
Hide resolved
packages/extension/src/providers/ecash/ui/send-transaction/index.vue
Outdated
Show resolved
Hide resolved
packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue
Outdated
Show resolved
Hide resolved
packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue
Outdated
Show resolved
Hide resolved
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue (1)
291-295:⚠️ Potential issue | 🟠 MajorFailure path uses inconsistent address key.
The success branch (line 276) stores activity under
displayFromAddress.value, but the failure path (line 293) usestxData.value.fromAddress. This causes failed transactions to be stored in a different bucket, making them disappear from the sender's history view.Proposed fix
txActivity.status = ActivityStatus.failed; activityState.addActivities([txActivity], { - address: txData.value.fromAddress, + address: displayFromAddress.value, network: network.value.name, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue` around lines 291 - 295, The failure branch sets txActivity.status = ActivityStatus.failed and calls activityState.addActivities with address: txData.value.fromAddress, which is inconsistent with the success branch that uses displayFromAddress.value; change the failure path to use the same address key (displayFromAddress.value) when calling activityState.addActivities so failed transactions are stored under the same sender bucket (update the call site where txActivity is added to use address: displayFromAddress.value).packages/extension/src/providers/ecash/ui/send-transaction/index.vue (1)
52-56:⚠️ Potential issue | 🟠 MajorHonor the selected fee tier end-to-end.
The selector is still pinned to
GasPriceTypes.REGULAR, and every downstream consumer reads the same constant. Fee changes never affect the balance/max-send checks or the verify payload, so the UI can show one tier while the transaction is prepared with another. Add a localselectedFeeref and use it as the single source of truth for the selector,currentGasFee, andtxVerifyInfo.gasPriceType.Also applies to: 188-190, 196-208, 313-320, 394-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue` around lines 52 - 56, Replace the hard-coded GasPriceTypes.REGULAR usage with a single reactive source-of-truth: create a ref named selectedFee (initialized to GasPriceTypes.REGULAR or the component prop/default), bind the send-fee-select :selected-fee to this selectedFee, and update all places that currently read GasPriceTypes.REGULAR (including currentGasFee computation, uses of gasCostValues[GasPriceTypes.REGULAR], txVerifyInfo.gasPriceType, and any balance/max-send checks) to read selectedFee instead; ensure txVerifyInfo.gasPriceType is written from selectedFee and derived currentGasFee uses gasCostValues[selectedFee] so the UI selector and all downstream transaction preparation stay consistent.
🧹 Nitpick comments (7)
packages/extension/src/providers/ecash/tests/utils.test.ts (1)
69-77: Inconsistent assertion style: mixingtoBe()andto.equal().The test file mixes Jest-style assertions (
.toBe()) with Chai-style assertions (.to.equal()). While Vitest supports both, consistency improves readability.Suggested change
it('should return Unknown for empty script', () => { - expect(scriptToAddress('')).to.equal('Unknown'); + expect(scriptToAddress('')).toBe('Unknown'); }); it('should return fallback for invalid script', () => { const invalidScript = 'invalid'; const result = scriptToAddress(invalidScript); - expect(result).to.equal('invalid'); + expect(result).toBe('invalid'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/tests/utils.test.ts` around lines 69 - 77, Tests in utils.test.ts mix Chai (`.to.equal()`) and Jest/Vitest (`.toBe()`) assertion styles causing inconsistency; pick one style and make all assertions consistent (e.g., change `expect(...).to.equal(...)` to `expect(...).toBe(...)` or vice versa) for the tests that exercise the scriptToAddress behavior (the two it blocks asserting Unknown and fallback for invalidScript) so both assertions use the same matcher style throughout the file.packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue (1)
103-103: Consider creating eCash-specific type instead of reusing Bitcoin's.
VerifyTransactionParamsis imported from@/providers/bitcoin/ui/types. If the structures diverge in future eCash features (e.g., eTokens), a dedicated type would prevent coupling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue` at line 103, The component currently imports VerifyTransactionParams from the Bitcoin provider which couples eCash UI to Bitcoin types; introduce a new eCash-specific type (e.g., EcashVerifyTransactionParams) in the eCash UI types module and replace the import and usage of VerifyTransactionParams in this component (and any eCash-specific verify-related props or methods) with the new EcashVerifyTransactionParams to decouple the types and allow future eCash extensions (like eTokens) without affecting Bitcoin types.packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts (1)
133-133: Hardcoded dust threshold should use network constant.The value
546is hardcoded here, butpackages/extension/src/providers/ecash/networks/ecash-base.tsdefinesdustas a configurable network option. Consider passing the dust threshold as a parameter for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts` at line 133, The conditional is using a hardcoded dust value (546) for leftover checks; replace that literal with the network's configured dust constant (exported as dust from ecash-base.ts) by accepting a dust parameter (number or BN) on the fee calculation function that uses leftover (where toBN(leftover) is compared) and using toBN(dust) in the comparison; update callers of that function to pass the network.dust value so the check uses the configurable network option instead of the magic number.packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue (2)
97-107: Clipboard fallback uses deprecated API andalert().
document.execCommand('paste')is deprecated and may not work in all browsers. Thealert()for permission denial is jarring UX. Consider using a toast notification or inline message instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue` around lines 97 - 107, The clipboard error handler currently uses alert() for NotAllowedError and the deprecated document.execCommand('paste') as a fallback; update the catch block in the send-address-input.vue component to replace alert('Please allow...') with a non-blocking UI notification (toast or inline error message component) tied to the component state, keep using addressInput.value?.focus() to move focus, and remove the document.execCommand('paste') fallback (or replace it with a clear instruction or UI paste button) so you no longer call the deprecated document.execCommand API; reference the existing DOMException/NotAllowedError check, addressInput ref, and the document.execCommand('paste') call when making the change.
55-60: Consider documenting the magic number 66.The threshold
props.value.length > 66appears to distinguish raw public keys from addresses. A brief comment would clarify intent for future maintainers.Suggested improvement
const xecAddress = computed(() => { + // Raw public keys (>66 chars) need conversion; CashAddr addresses pass through if (props.value && props.value.length > 66) { return props.network.displayAddress(props.value); } return props.value; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue` around lines 55 - 60, The condition using the magic number in the computed xecAddress should be documented and made clearer: in send-address-input.vue update the xecAddress computed block (the expression props.value.length > 66) by adding a short comment explaining that 66 is the maximum raw public-key length (so values longer than 66 are encoded addresses), and consider replacing the literal with a named constant (e.g., RAW_PUBKEY_MAX_LEN or PUBLIC_KEY_HEX_LEN) defined near the top of the file to clarify intent and improve maintainability.packages/extension/src/providers/ecash/libs/utils.ts (1)
37-38: Consider limiting logged script data.Logging the first 20 characters of a failed script may inadvertently expose sensitive information in error logs. Consider removing the script from the log message or using a hash instead.
Suggested change
} catch (error) { - console.error('[scriptToAddress] Error:', error, script.slice(0, 20)); + console.error('[scriptToAddress] Error converting script:', error);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/utils.ts` around lines 37 - 38, The catch block in scriptToAddress currently logs part of the script (console.error('[scriptToAddress] Error:', error, script.slice(0, 20))) which may expose sensitive data; remove the direct script data from the log and instead log a non-sensitive identifier such as a short hash (e.g., compute a SHA-256 or SHA-1 of script and log its first 8 chars) or only log the error message and context. Update the catch in scriptToAddress to stop printing script.slice(0,20) and either log a generated hash for the script or omit the script entirely, ensuring any hash-generation uses a standard crypto utility (crypto.createHash) elsewhere in the module if needed.packages/extension/src/providers/ecash/ui/send-transaction/index.vue (1)
285-289: Avoid the duplicate asset reload on sender changes.
selectAccountFrom()callsfetchAssets()even though thewatch([addressFrom], ...)block already doesfetchAssets().then(setBaseCosts)for the same update. That extra request can raceselectedAssetupdates and does not refresh UTXOs on its own, so it is better to keep the watcher as the single async path here.♻️ Minimal cleanup
const selectAccountFrom = (account: string) => { addressFrom.value = account; isOpenSelectContactFrom.value = false; - fetchAssets(); };Also applies to: 364-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue` around lines 285 - 289, The watcher on addressFrom already calls fetchAssets().then(setBaseCosts) to handle sender changes, so remove the duplicate fetchAssets() call from selectAccountFrom() to avoid racing selectedAsset updates and not-refreshing UTXOs; keep the watcher as the single async path, ensure selectAccountFrom() still updates the addressFrom ref/state and any synchronous logic, and apply the same cleanup for the other duplicate at the selectAccountFrom-related block around the second occurrence (lines 364–368) so only the watcher triggers fetchAssets and setBaseCosts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension/src/libs/background/internal/ecash-sign.ts`:
- Line 78: The line const amountBigInt = BigInt(params.amount) can throw for
invalid input; update the code around that statement (in ecash-sign.ts,
referencing params.amount and amountBigInt) to validate or safely parse the
amount before calling BigInt: either check the string matches a whole-integer
regex (e.g., /^\d+$/) and throw/return a clear user-facing error if not, or wrap
BigInt(params.amount) in a try-catch and convert caught errors into a
user-friendly error message that explains the amount must be an integer string.
In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts`:
- Around line 45-79: Determine isSend before isReceive: compute isSend by
checking tx.inputs for any input owned by normalizedAddress (using
scriptToAddress as currently done), then only set isReceive if isSend is false
and any tx.outputs belong to normalizedAddress; update the logic around the
isReceive/isSend flags in the txHistory loop (and the similar block at lines
~114-121) so calculateTransactionValue and getTransactionAddresses are called
with isReceive true only when no owned input exists, leaving change outputs
treated as part of outgoing transactions. Ensure the same ordering and check
change in both the first block (where isReceive/isSend are computed) and the
later duplicate block.
- Around line 90-98: The BTCRawInfo.inputs mapping is missing the pkscript field
expected by the BTCRawInfo type; update the inputs mapping inside rawInfo (the
block that constructs rawInfo.inputs) to include pkscript (use the same source
as outputs, e.g. input.outputScript or '' when absent) alongside address and
value so each input entry matches the BTC-like shape; ensure BTCRawInfo,
rawInfo.inputs and any consumers still compile with the added pkscript property
(functions referenced: scriptToAddress, extractSats).
In `@packages/extension/src/providers/ecash/libs/api-chronik.ts`:
- Around line 65-75: ChronikAPI.getBalance currently declares the parameter as
pubkey and calls getAddress(pubkey, this.networkInfo.cashAddrPrefix), which
violates the ProviderAPIInterface contract that the parameter is address; rename
the parameter to address in ChronikAPI.getBalance and stop treating it as a
pubkey, or if conversion is required accept both forms explicitly: detect
whether the incoming address is already a cashAddr and only call getAddress when
a pubkey is provided. Update references inside getBalance (ensurePrefix,
getAddress, WatchOnlyWallet.fromAddress, and the withErrorHandling call) so the
implementation matches the interface signature and correctly resolves balances
for callers passing an address.
In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts`:
- Around line 150-166: The function buildGasCostValues currently returns only
GasPriceTypes.REGULAR and casts to GasFeeType causing missing keys; update
buildGasCostValues to populate all required keys (GasPriceTypes.ECONOMY,
REGULAR, FAST, FASTEST) with appropriate entries (e.g., reuse the computed entry
or compute variants) so the returned object satisfies the GasFeeType shape
without using an unsafe cast; reference the existing buildGasCostValues
function, GasPriceTypes enum, and GasFeeType interface when adding the four
keys.
In `@packages/keyring/src/index.ts`:
- Around line 413-421: The .find() in the privkey branch can return undefined
and you unconditionally read .publicKey; change the code in the
WalletType.privkey branch to capture the result of await
this.getKeysArray().find(...) into a local variable (e.g., foundKey), check if
foundKey is undefined and handle it (throw a clear error or return/fallback)
before accessing foundKey.publicKey, then construct keypair using
this.#privkeys[account.pathIndex.toString()] and foundKey.publicKey so you avoid
a null dereference in the getKeysArray()/WalletType.privkey path.
---
Duplicate comments:
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 52-56: Replace the hard-coded GasPriceTypes.REGULAR usage with a
single reactive source-of-truth: create a ref named selectedFee (initialized to
GasPriceTypes.REGULAR or the component prop/default), bind the send-fee-select
:selected-fee to this selectedFee, and update all places that currently read
GasPriceTypes.REGULAR (including currentGasFee computation, uses of
gasCostValues[GasPriceTypes.REGULAR], txVerifyInfo.gasPriceType, and any
balance/max-send checks) to read selectedFee instead; ensure
txVerifyInfo.gasPriceType is written from selectedFee and derived currentGasFee
uses gasCostValues[selectedFee] so the UI selector and all downstream
transaction preparation stay consistent.
In
`@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue`:
- Around line 291-295: The failure branch sets txActivity.status =
ActivityStatus.failed and calls activityState.addActivities with address:
txData.value.fromAddress, which is inconsistent with the success branch that
uses displayFromAddress.value; change the failure path to use the same address
key (displayFromAddress.value) when calling activityState.addActivities so
failed transactions are stored under the same sender bucket (update the call
site where txActivity is added to use address: displayFromAddress.value).
---
Nitpick comments:
In `@packages/extension/src/providers/ecash/libs/utils.ts`:
- Around line 37-38: The catch block in scriptToAddress currently logs part of
the script (console.error('[scriptToAddress] Error:', error, script.slice(0,
20))) which may expose sensitive data; remove the direct script data from the
log and instead log a non-sensitive identifier such as a short hash (e.g.,
compute a SHA-256 or SHA-1 of script and log its first 8 chars) or only log the
error message and context. Update the catch in scriptToAddress to stop printing
script.slice(0,20) and either log a generated hash for the script or omit the
script entirely, ensuring any hash-generation uses a standard crypto utility
(crypto.createHash) elsewhere in the module if needed.
In `@packages/extension/src/providers/ecash/tests/utils.test.ts`:
- Around line 69-77: Tests in utils.test.ts mix Chai (`.to.equal()`) and
Jest/Vitest (`.toBe()`) assertion styles causing inconsistency; pick one style
and make all assertions consistent (e.g., change `expect(...).to.equal(...)` to
`expect(...).toBe(...)` or vice versa) for the tests that exercise the
scriptToAddress behavior (the two it blocks asserting Unknown and fallback for
invalidScript) so both assertions use the same matcher style throughout the
file.
In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts`:
- Line 133: The conditional is using a hardcoded dust value (546) for leftover
checks; replace that literal with the network's configured dust constant
(exported as dust from ecash-base.ts) by accepting a dust parameter (number or
BN) on the fee calculation function that uses leftover (where toBN(leftover) is
compared) and using toBN(dust) in the comparison; update callers of that
function to pass the network.dust value so the check uses the configurable
network option instead of the magic number.
In
`@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue`:
- Around line 97-107: The clipboard error handler currently uses alert() for
NotAllowedError and the deprecated document.execCommand('paste') as a fallback;
update the catch block in the send-address-input.vue component to replace
alert('Please allow...') with a non-blocking UI notification (toast or inline
error message component) tied to the component state, keep using
addressInput.value?.focus() to move focus, and remove the
document.execCommand('paste') fallback (or replace it with a clear instruction
or UI paste button) so you no longer call the deprecated document.execCommand
API; reference the existing DOMException/NotAllowedError check, addressInput
ref, and the document.execCommand('paste') call when making the change.
- Around line 55-60: The condition using the magic number in the computed
xecAddress should be documented and made clearer: in send-address-input.vue
update the xecAddress computed block (the expression props.value.length > 66) by
adding a short comment explaining that 66 is the maximum raw public-key length
(so values longer than 66 are encoded addresses), and consider replacing the
literal with a named constant (e.g., RAW_PUBKEY_MAX_LEN or PUBLIC_KEY_HEX_LEN)
defined near the top of the file to clarify intent and improve maintainability.
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 285-289: The watcher on addressFrom already calls
fetchAssets().then(setBaseCosts) to handle sender changes, so remove the
duplicate fetchAssets() call from selectAccountFrom() to avoid racing
selectedAsset updates and not-refreshing UTXOs; keep the watcher as the single
async path, ensure selectAccountFrom() still updates the addressFrom ref/state
and any synchronous logic, and apply the same cleanup for the other duplicate at
the selectAccountFrom-related block around the second occurrence (lines 364–368)
so only the watcher triggers fetchAssets and setBaseCosts.
In
`@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue`:
- Line 103: The component currently imports VerifyTransactionParams from the
Bitcoin provider which couples eCash UI to Bitcoin types; introduce a new
eCash-specific type (e.g., EcashVerifyTransactionParams) in the eCash UI types
module and replace the import and usage of VerifyTransactionParams in this
component (and any eCash-specific verify-related props or methods) with the new
EcashVerifyTransactionParams to decouple the types and allow future eCash
extensions (like eTokens) without affecting Bitcoin types.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 91baae14-ae4e-4c66-9a6c-0449e6972583
📒 Files selected for processing (15)
packages/extension/src/libs/activity-state/wrap-activity-handler.tspackages/extension/src/libs/background/index.tspackages/extension/src/libs/background/internal/ecash-sign.tspackages/extension/src/providers/ecash/libs/activity-handlers.tspackages/extension/src/providers/ecash/libs/api-chronik.tspackages/extension/src/providers/ecash/libs/utils.tspackages/extension/src/providers/ecash/tests/utils.test.tspackages/extension/src/providers/ecash/types/ecash-network.tspackages/extension/src/providers/ecash/ui/libs/fee-calculator.tspackages/extension/src/providers/ecash/ui/libs/send-utils.tspackages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vuepackages/extension/src/providers/ecash/ui/send-transaction/index.vuepackages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vuepackages/extension/src/ui/action/utils/filters.tspackages/keyring/src/index.ts
✅ Files skipped from review due to trivial changes (2)
- packages/extension/src/libs/activity-state/wrap-activity-handler.ts
- packages/extension/src/providers/ecash/ui/libs/send-utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/libs/background/index.ts
- packages/extension/src/ui/action/utils/filters.ts
packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts
Outdated
Show resolved
Hide resolved
Amatack
force-pushed
the
feature/ecash-integration
branch
from
e855326 to
6636777
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/extension/src/providers/ecash/libs/utils.ts (1)
64-70: Docstring parameter name mismatch.The docstring references
normalizedAddressbut the actual parameter isownedAddresses.📝 Proposed fix
/** * Calculate transaction value for receive or send * `@param` outputs - Array of transaction outputs - * `@param` normalizedAddress - The address to check against + * `@param` ownedAddresses - Array of owned addresses to check against * `@param` isReceive - true for received funds, false for sent funds * `@param` cashAddrPrefix - The cash address prefix (default: 'ecash') */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/utils.ts` around lines 64 - 70, The docstring for the transaction-value helper is out of sync: it mentions `normalizedAddress` while the actual parameter is `ownedAddresses`; update the comment to reference `ownedAddresses` (and briefly describe it as the array/set of addresses owned by the wallet to check against) and ensure other param descriptions (`outputs`, `isReceive`, `cashAddrPrefix`) remain correct for the function (look for the function that accepts ownedAddresses in this file to apply the change).packages/extension/src/providers/ecash/libs/api-chronik.ts (1)
148-168: Consider consolidating with the cachedscriptToAddressin utils.ts.This file has a local
scriptToAddressmethod that returns''on error, whileutils.tshas a cached version that returns a truncated script fallback. The implementations differ slightly:
- Here:
Address.fromScript(new Script(buffer), prefix)- utils.ts:
Address.fromScriptHex(scriptHex, prefix)with cachingIf the behaviors can be unified, consolidating would reduce duplication and ensure consistent caching across the codebase. However, if the different error behaviors are intentional (empty string here vs. truncated fallback in utils.ts for UI display), this is acceptable as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/api-chronik.ts` around lines 148 - 168, This file's private scriptToAddress duplicates logic from utils.ts (utils.scriptToAddress / Address.fromScriptHex) and returns an empty string on error instead of the utils.ts truncated-script fallback; to consolidate, replace this local scriptToAddress implementation with a call to the cached utils.scriptToAddress (import it) so caching and fallback behavior are consistent, or if the empty-string behavior is intentional, explicitly delegate to utils.scriptToAddress and map its result to '' when needed (i.e., call utils.scriptToAddress(scriptHex, this.networkInfo.cashAddrPrefix) inside the method or return its value directly) ensuring you reference the local method name scriptToAddress and the utils function Address.fromScriptHex / utils.scriptToAddress when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/extension/src/providers/ecash/libs/api-chronik.ts`:
- Around line 148-168: This file's private scriptToAddress duplicates logic from
utils.ts (utils.scriptToAddress / Address.fromScriptHex) and returns an empty
string on error instead of the utils.ts truncated-script fallback; to
consolidate, replace this local scriptToAddress implementation with a call to
the cached utils.scriptToAddress (import it) so caching and fallback behavior
are consistent, or if the empty-string behavior is intentional, explicitly
delegate to utils.scriptToAddress and map its result to '' when needed (i.e.,
call utils.scriptToAddress(scriptHex, this.networkInfo.cashAddrPrefix) inside
the method or return its value directly) ensuring you reference the local method
name scriptToAddress and the utils function Address.fromScriptHex /
utils.scriptToAddress when making the change.
In `@packages/extension/src/providers/ecash/libs/utils.ts`:
- Around line 64-70: The docstring for the transaction-value helper is out of
sync: it mentions `normalizedAddress` while the actual parameter is
`ownedAddresses`; update the comment to reference `ownedAddresses` (and briefly
describe it as the array/set of addresses owned by the wallet to check against)
and ensure other param descriptions (`outputs`, `isReceive`, `cashAddrPrefix`)
remain correct for the function (look for the function that accepts
ownedAddresses in this file to apply the change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00431653-2f20-414d-856a-cafd9be37345
📒 Files selected for processing (15)
packages/extension/src/libs/activity-state/wrap-activity-handler.tspackages/extension/src/libs/background/index.tspackages/extension/src/libs/background/internal/ecash-sign.tspackages/extension/src/providers/ecash/libs/activity-handlers.tspackages/extension/src/providers/ecash/libs/api-chronik.tspackages/extension/src/providers/ecash/libs/utils.tspackages/extension/src/providers/ecash/tests/utils.test.tspackages/extension/src/providers/ecash/types/ecash-network.tspackages/extension/src/providers/ecash/ui/libs/fee-calculator.tspackages/extension/src/providers/ecash/ui/libs/send-utils.tspackages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vuepackages/extension/src/providers/ecash/ui/send-transaction/index.vuepackages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vuepackages/extension/src/ui/action/utils/filters.tspackages/keyring/src/index.ts
✅ Files skipped from review due to trivial changes (1)
- packages/extension/src/providers/ecash/libs/activity-handlers.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/extension/src/libs/background/index.ts
- packages/extension/src/libs/activity-state/wrap-activity-handler.ts
- packages/extension/src/ui/action/utils/filters.ts
- packages/keyring/src/index.ts
- packages/extension/src/libs/background/internal/ecash-sign.ts
- packages/extension/src/providers/ecash/ui/libs/send-utils.ts
- packages/extension/src/providers/ecash/ui/send-transaction/index.vue
Amatack
force-pushed
the
feature/ecash-integration
branch
from
6636777 to
0029006
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue (1)
270-290:⚠️ Potential issue | 🟡 MinorStore failed sends under the same sender key as successful ones.
The success path writes with
displayFromAddress.value, but the catch block still buckets the failed activity undertxData.value.fromAddress. If that source is a raw account identifier, failed sends disappear from the sender's history.🩹 Minimal fix
txActivity.status = ActivityStatus.failed; activityState.addActivities([txActivity], { - address: txData.value.fromAddress, + address: displayFromAddress.value, network: network.value.name, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue` around lines 270 - 290, The catch block stores failed txActivity under a different sender key (txData.value.fromAddress) than the success path (displayFromAddress.value), causing failed sends to be missing from the sender's history; update the error handling to call activityState.addActivities([txActivity], { address: displayFromAddress.value, network: network.value.name }) so failures use the same sender key as the success path (ensure you update the catch block where txActivity is set to ActivityStatus.failed and activityState.addActivities is invoked).packages/extension/src/providers/ecash/libs/activity-handlers.ts (2)
90-103:⚠️ Potential issue | 🟠 MajorAdd
pkscripttorawInfo.inputs.
outputsalready carry the script, butinputsstill omit it here, so ECash activities expose a partial BTC-like raw payload.🩹 Minimal fix
inputs: tx.inputs.map((input: any) => ({ address: scriptToAddress(input.outputScript ?? '', cashAddrPrefix), value: Number(extractSats(input)), + pkscript: input.outputScript || '', })),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts` around lines 90 - 103, The rawInfo.inputs mapping omits the original script, causing incomplete BTC-like payloads; update the inputs creation inside rawInfo to include a pkscript field (similar to outputs) using the input.outputScript (or '' fallback) so each input object contains address, value and pkscript; locate the BTCRawInfo rawInfo construction (the inputs: tx.inputs.map(...) block) and add pkscript: input.outputScript || '' alongside the existing address and value fields (helpers: scriptToAddress, extractSats).
45-79:⚠️ Potential issue | 🟠 MajorTreat change as part of an outgoing tx, not a receive.
A normal send with change sets both flags here, so
isIncomingbecomestrueand the value/address helpers run in receive mode. ComputeisSendfirst and only allowisReceivewhen no owned input exists.🩹 Minimal fix
- const isReceive = tx.outputs.some((output: any) => { - const outputAddress = scriptToAddress( - output.outputScript, - cashAddrPrefix, - ); - return outputAddress === normalizedAddress; - }); - const isSend = tx.inputs.some((input: any) => { const inputAddress = scriptToAddress( input.outputScript ?? '', cashAddrPrefix, ); return inputAddress === normalizedAddress; }); + + const isReceive = !isSend && tx.outputs.some((output: any) => { + const outputAddress = scriptToAddress( + output.outputScript, + cashAddrPrefix, + ); + return outputAddress === normalizedAddress; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts` around lines 45 - 79, In the txHistory loop the code currently computes isReceive and isSend independently which makes transactions with owned inputs+outputs (sends with change) treated as incoming; change the logic to compute isSend first by checking tx.inputs for any scriptToAddress matching normalizedAddress, then compute isReceive only when no owned input exists (isReceive = !isSend && outputs.some(...)); update subsequent uses of isReceive/isSend (calls to calculateTransactionValue and getTransactionAddresses) to rely on this ordering so outgoing transactions with change are treated as sends.packages/extension/src/libs/background/internal/ecash-sign.ts (1)
78-92:⚠️ Potential issue | 🟡 MinorValidate
amountas a positive integer string beforeBigInt().
BigInt()throws on decimal/non-numeric input, and zero or negative values still reach wallet construction here. Reject anything that is not a positive whole-sat amount up front so the user gets a controlled error.🩹 Minimal fix
- const amountBigInt = BigInt(params.amount); + if (!/^[1-9]\d*$/.test(params.amount)) { + throw new Error('Invalid amount: must be a positive integer string'); + } + const amountBigInt = BigInt(params.amount);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/libs/background/internal/ecash-sign.ts` around lines 78 - 92, The code converts params.amount to BigInt without validating input, which allows decimals, negatives or non-numeric strings to throw or pass through; before calling BigInt(params.amount) validate params.amount is a string of only digits (e.g. /^\d+$/) and not "0", then convert to BigInt and proceed; if validation fails throw a clear Error (e.g. "Invalid amount: must be a positive integer number of sats") so wallet.spendableSatsOnlyUtxos(), amountBigInt and wallet.action(...) only receive a validated positive whole-sat amount.packages/extension/src/providers/ecash/ui/send-transaction/index.vue (1)
52-56:⚠️ Potential issue | 🟠 MajorMake the fee selector drive
currentGasFeeandgasPriceType.This component is still hard-wired to
GasPriceTypes.REGULAR, so changing the fee tier in the UI never affects the displayed fee, max amount, or the verify payload.Also applies to: 188-190, 313-320, 394-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue` around lines 52 - 56, The send-fee-select is hardcoded to GasPriceTypes.REGULAR so UI changes don't update state; change the component bindings to use the reactive gasPriceType and currentGasFee (and gasCostValues[gasPriceType]) instead of the constant: update the :selected-fee prop to bind gasPriceType, the :fee prop to bind gasCostValues[gasPriceType] (or currentGasFee if you compute it), and ensure the send-fee-select change handler updates both gasPriceType and currentGasFee so downstream logic (max amount calculation and verify payload in functions that reference currentGasFee/gasPriceType) uses the selected tier. Apply the same replacement for the other occurrences around the referenced blocks (lines ~188-190, 313-320, 394-411) so all UI instances drive and reflect the shared gasPriceType/currentGasFee state.
🧹 Nitpick comments (2)
packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue (2)
55-60: Consider adding a comment for the magic number 66.The threshold
66represents a compressed secp256k1 public key length in hex (33 bytes × 2). A brief comment would help future maintainers understand the pubkey-vs-address distinction.Suggested clarification
+// Compressed secp256k1 pubkeys are 66 hex chars; CashAddr addresses are shorter const xecAddress = computed(() => { if (props.value && props.value.length > 66) { return props.network.displayAddress(props.value); } return props.value; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue` around lines 55 - 60, The magic number 66 in the xecAddress computed is unclear: add a concise inline comment next to the length check (props.value && props.value.length > 66) explaining that 66 is the hex length of a compressed secp256k1 public key (33 bytes × 2), and that values longer than this are treated as raw pubkeys and must be converted via network.displayAddress(props.value); keep the comment short and reference the pubkey-vs-address distinction so future readers understand the threshold.
80-109: Minor inconsistency between clipboard paths;execCommandis deprecated.Two observations:
- The Clipboard API path trims the text (line 94), but the
execCommand('paste')fallback pastes directly without trimming.document.execCommand('paste')is deprecated and may be removed from browsers.These are minor concerns since the fallback handles legacy cases and whitespace in addresses is uncommon.
Optional: Add note about deprecation
console.warn( '⚠️ Clipboard API not supported, falling back to execCommand', ); addressInput.value?.focus(); + // Note: execCommand('paste') is deprecated but serves as legacy fallback document.execCommand('paste'); return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue` around lines 80 - 109, The pasteFromClipboard handler uses navigator.clipboard.readText() and trims the result but falls back to deprecated document.execCommand('paste') without trimming and without warning; update pasteFromClipboard to (1) after the execCommand fallback, read the current value from the address input element (addressInput) and emit the trimmed value via emit('update:inputAddress', ...) instead of relying on execCommand alone, and (2) log a clear deprecation warning when using document.execCommand('paste') so future maintainers know it’s legacy and may be removed. Ensure you reference the pasteFromClipboard function, addressInput ref and the emit('update:inputAddress', ...) call when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension/src/providers/ecash/libs/api-chronik.ts`:
- Around line 113-118: The timestamp fallback in getTransactionStatus currently
uses Math.floor(Date.now() / 1000) causing unconfirmed transactions to get a new
timestamp each refresh; update the BTCRawInfo timestamp assignment in the
getTransactionStatus flow to use getTransactionTimestamp(tx) (the same helper
used by the activity path) instead of Date.now() so pending tx timestamps remain
stable across refreshes.
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 384-425: The sendAction handler dereferences getAccount() result
(fromAccountInfo.address, .name, .isHardware) without checking for null; update
sendAction to guard the PublicKeyRing.getAccount(addressFrom.value) return
(fromAccountInfo) — if it is null/undefined, show a user-facing
error/notification and return early (or route back) instead of proceeding; when
building txVerifyInfo and the subsequent routing use the guarded fromAccountInfo
values (or safe fallbacks) and only access
fromAddress/fromAddressName/isHardware after confirming fromAccountInfo exists.
- Around line 246-255: The computed sendButtonTitle uses parseInt on
sendAmount.value which treats fractional amounts like "0.01" as 0; change the
threshold check to a decimal-safe numeric comparison (e.g., use
Number(sendAmount.value) or parseFloat(sendAmount.value) or a BigNumber/Decimal
library) so fractions > 0 are detected; update the computed sendButtonTitle
(referencing sendButtonTitle, sendAmount, formatFloatingPointValue, and
selectedAsset) to use the decimal-safe check before building the "Send X SYMBOL"
string.
---
Duplicate comments:
In `@packages/extension/src/libs/background/internal/ecash-sign.ts`:
- Around line 78-92: The code converts params.amount to BigInt without
validating input, which allows decimals, negatives or non-numeric strings to
throw or pass through; before calling BigInt(params.amount) validate
params.amount is a string of only digits (e.g. /^\d+$/) and not "0", then
convert to BigInt and proceed; if validation fails throw a clear Error (e.g.
"Invalid amount: must be a positive integer number of sats") so
wallet.spendableSatsOnlyUtxos(), amountBigInt and wallet.action(...) only
receive a validated positive whole-sat amount.
In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts`:
- Around line 90-103: The rawInfo.inputs mapping omits the original script,
causing incomplete BTC-like payloads; update the inputs creation inside rawInfo
to include a pkscript field (similar to outputs) using the input.outputScript
(or '' fallback) so each input object contains address, value and pkscript;
locate the BTCRawInfo rawInfo construction (the inputs: tx.inputs.map(...)
block) and add pkscript: input.outputScript || '' alongside the existing address
and value fields (helpers: scriptToAddress, extractSats).
- Around line 45-79: In the txHistory loop the code currently computes isReceive
and isSend independently which makes transactions with owned inputs+outputs
(sends with change) treated as incoming; change the logic to compute isSend
first by checking tx.inputs for any scriptToAddress matching normalizedAddress,
then compute isReceive only when no owned input exists (isReceive = !isSend &&
outputs.some(...)); update subsequent uses of isReceive/isSend (calls to
calculateTransactionValue and getTransactionAddresses) to rely on this ordering
so outgoing transactions with change are treated as sends.
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 52-56: The send-fee-select is hardcoded to GasPriceTypes.REGULAR
so UI changes don't update state; change the component bindings to use the
reactive gasPriceType and currentGasFee (and gasCostValues[gasPriceType])
instead of the constant: update the :selected-fee prop to bind gasPriceType, the
:fee prop to bind gasCostValues[gasPriceType] (or currentGasFee if you compute
it), and ensure the send-fee-select change handler updates both gasPriceType and
currentGasFee so downstream logic (max amount calculation and verify payload in
functions that reference currentGasFee/gasPriceType) uses the selected tier.
Apply the same replacement for the other occurrences around the referenced
blocks (lines ~188-190, 313-320, 394-411) so all UI instances drive and reflect
the shared gasPriceType/currentGasFee state.
In
`@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue`:
- Around line 270-290: The catch block stores failed txActivity under a
different sender key (txData.value.fromAddress) than the success path
(displayFromAddress.value), causing failed sends to be missing from the sender's
history; update the error handling to call
activityState.addActivities([txActivity], { address: displayFromAddress.value,
network: network.value.name }) so failures use the same sender key as the
success path (ensure you update the catch block where txActivity is set to
ActivityStatus.failed and activityState.addActivities is invoked).
---
Nitpick comments:
In
`@packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue`:
- Around line 55-60: The magic number 66 in the xecAddress computed is unclear:
add a concise inline comment next to the length check (props.value &&
props.value.length > 66) explaining that 66 is the hex length of a compressed
secp256k1 public key (33 bytes × 2), and that values longer than this are
treated as raw pubkeys and must be converted via
network.displayAddress(props.value); keep the comment short and reference the
pubkey-vs-address distinction so future readers understand the threshold.
- Around line 80-109: The pasteFromClipboard handler uses
navigator.clipboard.readText() and trims the result but falls back to deprecated
document.execCommand('paste') without trimming and without warning; update
pasteFromClipboard to (1) after the execCommand fallback, read the current value
from the address input element (addressInput) and emit the trimmed value via
emit('update:inputAddress', ...) instead of relying on execCommand alone, and
(2) log a clear deprecation warning when using document.execCommand('paste') so
future maintainers know it’s legacy and may be removed. Ensure you reference the
pasteFromClipboard function, addressInput ref and the
emit('update:inputAddress', ...) call when making the change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ce77edc-8550-48b7-bbda-5d642400cb4b
📒 Files selected for processing (10)
packages/extension/src/libs/activity-state/wrap-activity-handler.tspackages/extension/src/libs/background/index.tspackages/extension/src/libs/background/internal/ecash-sign.tspackages/extension/src/providers/ecash/libs/activity-handlers.tspackages/extension/src/providers/ecash/libs/api-chronik.tspackages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vuepackages/extension/src/providers/ecash/ui/send-transaction/index.vuepackages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vuepackages/extension/src/ui/action/utils/filters.tspackages/keyring/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/libs/activity-state/wrap-activity-handler.ts
- packages/keyring/src/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds native eCash (XEC) support to enKrypt as a new provider.
Technical Details
Blockbook, required for Avalanche pre-consensus finality and future
eToken support
ecash-libm/44'/1899'/0', just like in cashtab walletFeatures
Out of scope (follow-up PR)
eToken support (SLP/ALP) is intentionally excluded from this PR to
keep the review focused. It will be submitted as a follow-up that
depends on this branch.
Testing
These test files are included covering address derivation, utility
functions, transaction signing, and keyring integration.
ecash.address.derivation.test.tsVerifies that a secp256k1 public key correctly derives a mainnet
CashAddr address via
Address.p2pkh.utils.test.tsCovers the core utility functions:
isValidECashAddress(with/withoutprefix, invalid inputs),
scriptToAddress(P2PKH hex, empty/invalidscripts, truncation),
extractSats,sumSatoshis(including largevalues and empty arrays),
calculateTransactionValue,calculateOnchainTxFee,getTransactionAddresses, andgetTransactionTimestamp(block timestamp,timeFirstSeen, andunconfirmed fallback).
ecash-sign.test.tsCovers the full signing flow: successful broadcast with txid
verification, insufficient balance, broadcast failures (with and
without error messages from the node),
wallet.sync()exceptions,getPrivateKeyForECashexceptions, locked keyring, invaliddestination address, missing params, unknown network, and hardware
wallet rejection.
generate.test.ts(keyring)Verifies deterministic
secp256k1ecashkey generation from mnemonicat path
m/44'/1899'/0'/0, and that a BIP39 passphrase produces adistinct public key.
Summary by CodeRabbit