Conversation
…and UI components
WalkthroughComprehensive eCash (XEC) blockchain network support is added to the extension, including dependencies, mainnet/testnet network configuration, Chronik API integration, transaction signing, activity tracking, and send/verify UI flows. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SendUI as Send UI<br/>(Vue)
participant BackgroundHandler as Background<br/>Handler
participant Keyring
participant Chronik as Chronik API<br/>(wallet)
participant Network as eCash<br/>Network
User->>SendUI: Click "Send" with amount
SendUI->>SendUI: Validate address, amount, balance
SendUI->>User: Show verify screen
User->>SendUI: Confirm transaction
SendUI->>BackgroundHandler: InternalMethods.ecashSign{fromAddr, toAddr, amount}
BackgroundHandler->>Network: getNetwork(name)
BackgroundHandler->>Keyring: getPrivateKeyForECash(account)
Keyring-->>BackgroundHandler: privateKeyBuffer
BackgroundHandler->>Chronik: new ChronikClient(node)
BackgroundHandler->>Chronik: wallet.sync()
Chronik-->>BackgroundHandler: walletState
BackgroundHandler->>Chronik: Sum spendable satoshis
Note over BackgroundHandler: Verify amount ≤ balance
BackgroundHandler->>Chronik: wallet.action().to(toAddr).send(sats)
BackgroundHandler->>Chronik: wallet.build()
BackgroundHandler->>Chronik: wallet.broadcast()
Chronik-->>BackgroundHandler: txid
BackgroundHandler->>Keyring: Zero-fill privateKey
BackgroundHandler-->>SendUI: { result: {txid} }
SendUI->>SendUI: Parse txid
SendUI->>User: Show success, close window
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 |
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
| const txid = result.broadcasted[0] || ''; | ||
| return { result: JSON.stringify({ txid }) }; |
There was a problem hiding this comment.
Handle empty broadcasted array explicitly.
If result.success is true but result.broadcasted is an empty array, txid will be an empty string. This edge case could confuse downstream consumers expecting a valid transaction ID on success.
Proposed fix
- const txid = result.broadcasted[0] || '';
- return { result: JSON.stringify({ txid }) };
+ const txid = result.broadcasted[0];
+ if (!txid) {
+ throw new Error('Broadcast succeeded but no txid returned');
+ }
+ return { result: JSON.stringify({ txid }) };📝 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 txid = result.broadcasted[0] || ''; | |
| return { result: JSON.stringify({ txid }) }; | |
| const txid = result.broadcasted[0]; | |
| if (!txid) { | |
| throw new Error('Broadcast succeeded but no txid returned'); | |
| } | |
| return { result: JSON.stringify({ txid }) }; |
🤖 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
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.
| async getBalance(pubkey: string): Promise<string> { | ||
| return this.withErrorHandling( | ||
| 'getBalance', | ||
| async () => { | ||
| const address = getAddress(pubkey); | ||
| const wallet = WatchOnlyWallet.fromAddress(address, this.chronik); | ||
| await wallet.sync(); | ||
| return wallet.balanceSats.toString(); | ||
| }, | ||
| () => '0', | ||
| ); |
There was a problem hiding this comment.
Respect the configured cashAddrPrefix when deriving watch-only addresses.
getBalance() currently calls getAddress(pubkey) with the mainnet default. On testnet that derives an ecash address even though this ChronikAPI instance was created with a different networkInfo.cashAddrPrefix, so WatchOnlyWallet.fromAddress() can query the wrong chain.
🩹 Minimal fix
- const address = getAddress(pubkey);
+ const address = this.ensurePrefix(
+ getAddress(pubkey, this.networkInfo.cashAddrPrefix),
+ );
const wallet = WatchOnlyWallet.fromAddress(address, this.chronik);🤖 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 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.
| export const isValidECashAddress = (address: string): boolean => { | ||
| try { | ||
| const addr = Address.parse(address); | ||
| return Boolean(addr); | ||
| } catch { | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Make address validation network-aware.
isValidECashAddress() only checks that the string is parseable, while getAddressWithoutPrefix() removes the only network discriminator afterwards. With testnet support in the same PR, that lets wrong-network addresses get accepted and then silently normalized instead of rejected.
Also applies to: 134-138
🤖 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 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.
| const sendAction = async () => { | ||
| await recentlySentAddresses.addRecentlySentAddress( | ||
| props.network, | ||
| addressTo.value, | ||
| ); |
There was a problem hiding this comment.
Persist recent recipients only after a successful broadcast.
This write happens before the verify step and before InternalMethods.ecashSign returns a txid, so cancelled or failed sends still end up in the recent-address list. Move it into the success path in packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue:214-238.
🤖 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 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.
| const selectedNetwork: string = route.query.id as string; | ||
|
|
||
| const txData: VerifyTransactionParams = JSON.parse( | ||
| Buffer.from(route.query.txData as string, 'base64').toString('utf8'), | ||
| ); |
There was a problem hiding this comment.
Guard route payload decoding before setup continues.
route.query.txData is parsed immediately, and route.query.id / KeyRing.getAccount() are consumed without validation. A missing or malformed query string currently throws before the component can render, and an unknown network/account leaves the user stuck on the loading state instead of getting a recoverable error. Please move the decode into onBeforeMount with try/catch and an explicit error state.
Also applies to: 138-145
🤖 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 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.
| activityState.addActivities( | ||
| [ | ||
| { | ||
| ...txActivity, | ||
| transactionHash: txid, | ||
| }, | ||
| ], | ||
| { | ||
| address: displayFromAddress.value, | ||
| network: network.value.name, | ||
| }, |
There was a problem hiding this comment.
Use the normalized sender key in the failure path too.
The success branch writes activities under displayFromAddress.value, but the catch block uses txData.fromAddress. If txData.fromAddress is still a raw account identifier, failed sends are stored under a different bucket and disappear from the sender's history.
🩹 Minimal fix
txActivity.status = ActivityStatus.failed;
activityState.addActivities([txActivity], {
- address: txData.fromAddress,
+ address: displayFromAddress.value,
network: network.value.name,
});Also applies to: 245-249
🤖 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 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.
| let minimumFractionDigits = 2; | ||
| let maximumFractionDigits = 2; | ||
|
|
||
| if (finalValue > 0 && finalValue < 0.01) { | ||
| minimumFractionDigits = 2; | ||
| maximumFractionDigits = 8; | ||
| } else if (finalValue >= 0.01 && finalValue < 1) { | ||
| minimumFractionDigits = 2; | ||
| maximumFractionDigits = 4; | ||
| } | ||
|
|
||
| const formatted = new Intl.NumberFormat(locale, { | ||
| style: 'currency', | ||
| currency: currency, | ||
| notation, | ||
| minimumFractionDigits, | ||
| maximumFractionDigits, | ||
| }).format(finalValue); | ||
|
|
||
| return `${amount.lt(0.0000001) && amount.gt(0) ? '< ' : ''}${formatted}`; |
There was a problem hiding this comment.
Tie the < marker to formatter rounding, not the old fixed threshold.
With maximumFractionDigits now reaching 8, Line 61 can prepend < to values that already render exactly, e.g. 0.00000005 → < $0.00000005. Compute the marker from the rounded formatter output instead of amount < 1e-7.
Proposed fix
- const formatted = new Intl.NumberFormat(locale, {
+ const formatter = new Intl.NumberFormat(locale, {
style: 'currency',
currency: currency,
notation,
minimumFractionDigits,
maximumFractionDigits,
- }).format(finalValue);
+ });
+ const formatted = formatter.format(finalValue);
+ const zeroFormatted = formatter.format(0);
- return `${amount.lt(0.0000001) && amount.gt(0) ? '< ' : ''}${formatted}`;
+ return `${finalValue > 0 && formatted === zeroFormatted ? '< ' : ''}${formatted}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/extension/src/ui/action/utils/filters.ts` around lines 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.
| async getPrivateKeyForECash(account: EnkryptAccount): Promise<Buffer> { | ||
| assert(!this.#isLocked, Errors.KeyringErrors.Locked); | ||
| this.#resetTimeout(); | ||
| assert( | ||
| account.signerType === SignerType.secp256k1ecash, | ||
| Errors.KeyringErrors.CannotUseKeyring, | ||
| ); | ||
| assert( | ||
| !Object.values(HWwalletType).includes( | ||
| account.walletType as unknown as HWwalletType, | ||
| ), | ||
| Errors.KeyringErrors.CannotUseKeyring, | ||
| ); | ||
|
|
||
| let keypair: KeyPair; | ||
| if (account.walletType === WalletType.privkey) { | ||
| const pubKey = (await this.getKeysArray()).find( | ||
| (i) => | ||
| i.basePath === account.basePath && i.pathIndex === account.pathIndex, | ||
| ).publicKey; | ||
| keypair = { | ||
| privateKey: this.#privkeys[account.pathIndex.toString()], | ||
| publicKey: pubKey, | ||
| }; | ||
| } else { | ||
| keypair = await this.#signers[account.signerType].generate( | ||
| this.#mnemonic, | ||
| pathParser(account.basePath, account.pathIndex, account.signerType), | ||
| ); | ||
| } | ||
|
|
||
| return hexToBuffer(keypair.privateKey); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "BitcoinSigner privateKey format:"
fd -e ts -e js . packages | rg 'signer-bitcoin' | xargs -r rg -n -C4 'class BitcoinSigner|generate\(|privateKey|toWIF|toWif|wif'
echo
echo "eCash private-key import and call sites:"
rg -n -C3 'SignerType\.secp256k1ecash|WalletType\.privkey|addKeyPair\(' packagesRepository: enkryptcom/enKrypt
Length of output: 94
🏁 Script executed:
find packages -type f -name "*.ts" -o -name "*.js" | head -20Repository: enkryptcom/enKrypt
Length of output: 1000
🏁 Script executed:
rg -l "BitcoinSigner" packagesRepository: enkryptcom/enKrypt
Length of output: 221
🏁 Script executed:
rg -l "getPrivateKeyForECash" packagesRepository: enkryptcom/enKrypt
Length of output: 267
🏁 Script executed:
rg -l "secp256k1ecash" packagesRepository: enkryptcom/enKrypt
Length of output: 288
🏁 Script executed:
cat -n packages/signers/bitcoin/src/index.ts | head -100Repository: enkryptcom/enKrypt
Length of output: 2110
🏁 Script executed:
wc -l packages/signers/bitcoin/src/index.tsRepository: enkryptcom/enKrypt
Length of output: 102
🏁 Script executed:
rg -n "generate\(" packages/signers/bitcoin/src/index.ts -A 20Repository: enkryptcom/enKrypt
Length of output: 738
🏁 Script executed:
rg -n "class BitcoinSigner" packages/signers/bitcoin/src/index.ts -A 50Repository: enkryptcom/enKrypt
Length of output: 1550
🏁 Script executed:
rg -n "addKeyPair\(" packages/keyring/src/index.ts -A 30Repository: enkryptcom/enKrypt
Length of output: 1086
🏁 Script executed:
sed -n '398,429p' packages/keyring/src/index.tsRepository: enkryptcom/enKrypt
Length of output: 1121
🏁 Script executed:
rg -n "this\.#privkeys\[" packages/keyring/src/index.tsRepository: enkryptcom/enKrypt
Length of output: 201
🏁 Script executed:
rg -n "#setPrivateKey" packages/keyring/src/index.ts -B 2 -A 20Repository: enkryptcom/enKrypt
Length of output: 1459
🏁 Script executed:
rg -n "this\.#privkeys\s*=" packages/keyring/src/index.ts -B 5 -A 5Repository: enkryptcom/enKrypt
Length of output: 1489
🏁 Script executed:
rg -n "getKeysArray" packages/keyring/src/index.ts -A 15 | head -50Repository: enkryptcom/enKrypt
Length of output: 1932
🏁 Script executed:
cat packages/keyring/tests/generate.test.tsRepository: enkryptcom/enKrypt
Length of output: 15603
🏁 Script executed:
rg -n "addKeyPair" packages/extension/src/libs/keyring/keyring.ts -B 5 -A 10Repository: enkryptcom/enKrypt
Length of output: 631
🏁 Script executed:
cat packages/extension/src/libs/background/internal/ecash-sign.tsRepository: enkryptcom/enKrypt
Length of output: 3242
Validate imported private-key encoding before hexToBuffer() conversion.
getPrivateKeyForECash() assumes all private keys are hex-encoded: it unconditionally calls hexToBuffer(keypair.privateKey) at line 429. For mnemonic-derived keys, BitcoinSigner.generate() guarantees raw hex (bufferToHex at line 22), so they are safe. However, for imported keys (WalletType.privkey), addKeyPair() stores whatever format the caller provides without validation. If an imported secp256k1ecash key is passed in WIF, raw bytes, or another non-hex format, hexToBuffer() will silently produce incorrect bytes, breaking the ecash-wallet signature operation.
Add format validation in addKeyPair() or #setPrivateKey() to normalize all imported private keys to hex, or validate their encoding before getPrivateKeyForECash() calls hexToBuffer().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/keyring/src/index.ts` around lines 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.
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
Release Notes