Feature: eCash integration#790
Open
Amatack wants to merge 14 commits intoenkryptcom:mainfrom
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces comprehensive eCash (XEC) mainnet and testnet support to the wallet extension, including network configuration, transaction signing via the background RPC handler, activity history fetching through Chronik API, send/verify transaction UI flows, utility functions for address validation and fee calculation, and necessary keyring extensions for private key retrieval. Changes
Sequence DiagramsequenceDiagram
participant User
participant SendUI as Send UI (index.vue)
participant VerifyUI as Verify UI (index.vue)
participant BG as Background Handler
participant Keyring
participant Wallet as ecash-wallet
participant Chronik
participant Network as Node
User->>SendUI: Enter recipient & amount
SendUI->>SendUI: Validate address & compute fee
SendUI->>VerifyUI: Route with tx params (base64)
User->>VerifyUI: Confirm transaction
VerifyUI->>BG: sendUsingInternalMessengers(ecashSign)
BG->>Keyring: getPrivateKeyForECash(account)
Keyring-->>BG: privateKey Buffer
BG->>Wallet: construct from privateKey
BG->>Wallet: sync() - fetch UTXOs
Wallet->>Chronik: utxos(address)
Chronik->>Network: query
Network-->>Chronik: UTXO list
Chronik-->>Wallet: UTXO response
BG->>Wallet: action(outputs)
BG->>Wallet: build()
Wallet-->>BG: unsignedTx
BG->>Wallet: broadcast(tx)
Wallet->>Chronik: broadcastTx
Chronik->>Network: submit
Network-->>Chronik: txid
Chronik-->>Wallet: success + txid
BG->>BG: zero private key buffer
BG-->>VerifyUI: {result: txid}
VerifyUI->>VerifyUI: Record Activity
VerifyUI-->>User: Transaction confirmed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
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
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
Amatack
force-pushed
the
feature/ecash-integration
branch
from
0029006 to
eefeb23
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/extension/src/providers/solana/ui/send-transaction/index.vue (1)
455-474:⚠️ Potential issue | 🟠 MajorAlways clear loading state in
fetchAssetsfailure pathsLine 458 turns loading on, but only the success branch (Line 473) turns it off. If token loading fails, the screen can remain stuck in the loading state. Please also guard empty
allAssetsbefore assigningselectedAsset.🛠️ Suggested fix
const fetchAssets = () => { accountAssets.value = []; selectedAsset.value = loadingAsset; isLoadingAssets.value = true; - return props.network.getAllTokens(addressFrom.value).then(allAssets => { - accountAssets.value = allAssets as SOLToken[]; - const hasParamData = - isSendToken.value && tokenParamData && tokenParamData.contract; - if (hasParamData) { - const selectedToken = accountAssets.value.find( - asset => asset.contract === tokenParamData.contract, - ); - if (selectedToken) { - selectedAsset.value = selectedToken as SOLToken; - } - } else { - selectedAsset.value = allAssets[0] as SOLToken; - } - isLoadingAssets.value = false; - }); + return props.network + .getAllTokens(addressFrom.value) + .then(allAssets => { + accountAssets.value = allAssets as SOLToken[]; + const firstAsset = accountAssets.value[0]; + const hasParamData = + isSendToken.value && tokenParamData && tokenParamData.contract; + if (hasParamData) { + const selectedToken = accountAssets.value.find( + asset => asset.contract === tokenParamData.contract, + ); + selectedAsset.value = (selectedToken as SOLToken) ?? loadingAsset; + } else { + selectedAsset.value = (firstAsset as SOLToken) ?? loadingAsset; + } + }) + .catch(() => { + accountAssets.value = []; + selectedAsset.value = loadingAsset; + }) + .finally(() => { + isLoadingAssets.value = false; + }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/solana/ui/send-transaction/index.vue` around lines 455 - 474, The fetchAssets function sets isLoadingAssets true but only clears it on the successful promise path; update fetchAssets (the promise returned from props.network.getAllTokens inside fetchAssets) to handle errors and always clear isLoadingAssets (use .catch or .finally) so the UI never stays stuck, and on failure reset accountAssets and selectedAsset to fallback (e.g., loadingAsset) to avoid stale state; additionally, before assigning selectedAsset = allAssets[0] or using .find against tokenParamData.contract, guard that allAssets is non-empty/defined to avoid undefined assignments.packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
310-317:⚠️ Potential issue | 🟠 MajorHarden
fetchAssetsto prevent permanent loading lockWith Line 312 setting
isLoadingAssets = true, failure ofgetAllTokenskeeps the UI blocked because loading is cleared only in the success callback. Please addcatch/finallyand a fallback for empty assets.🛠️ Suggested fix
const fetchAssets = () => { selectedAsset.value = loadingAsset; isLoadingAssets.value = true; - return props.network.getAllTokens(addressFrom.value).then(allAssets => { - accountAssets.value = allAssets as BTCToken[]; - selectedAsset.value = allAssets[0] as BTCToken; - isLoadingAssets.value = false; - }); + return props.network + .getAllTokens(addressFrom.value) + .then(allAssets => { + accountAssets.value = allAssets as BTCToken[]; + selectedAsset.value = (accountAssets.value[0] as BTCToken) ?? loadingAsset; + }) + .catch(() => { + accountAssets.value = []; + selectedAsset.value = loadingAsset; + }) + .finally(() => { + isLoadingAssets.value = false; + }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue` around lines 310 - 317, In fetchAssets, ensure we always clear the loading flag and provide a fallback asset on error: wrap props.network.getAllTokens(addressFrom.value) with .catch and .finally, set accountAssets.value = [] and selectedAsset.value = loadingAsset in the .catch handler (or if allAssets is empty), and move isLoadingAssets.value = false into .finally so the UI won't stay blocked; keep existing success path assigning accountAssets.value and selectedAsset.value when tokens are returned.packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-token-select.vue (1)
2-17:⚠️ Potential issue | 🟠 MajorUse a semantic button for the token selector trigger
Line 2 uses an anchor element as an action control (
@click) without anhrefattribute, which breaks keyboard accessibility. Anchor tags withouthrefare not keyboard-focusable by default and lack proper button semantics. Replace with a<button type="button">instead.♿ Suggested fix
- <a class="send-token-select" `@click`="emit('update:toggleTokenSelect')"> + <button + type="button" + class="send-token-select" + `@click`="emit('update:toggleTokenSelect')" + > @@ - </a> + </button>.send-token-select { + appearance: none; + text-align: left; height: 60px;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-token-select.vue` around lines 2 - 17, Replace the non-semantic anchor used as the click trigger with a proper button element to restore keyboard accessibility: change the root element of the SendTokenSelect component (the element with class "send-token-select" and the click handler that calls emit('update:toggleTokenSelect')) to a <button type="button">, keep the same classes and children (send-token-select__image, send-token-select__info, send-token-select__arrow, <switch-arrow/>) and ensure any styling or role-dependent CSS still applies; verify the click handler and any event emit logic remains identical so update:toggleTokenSelect continues to fire.
♻️ Duplicate comments (5)
packages/keyring/src/index.ts (1)
413-421:⚠️ Potential issue | 🟡 MinorPotential null dereference if account not found in keys array.
The
.find()call on line 414 may returnundefinedif no matching account exists, but line 417 accesses.publicKeyunconditionally. This would throw a runtime error.Proposed fix
if (account.walletType === WalletType.privkey) { - const pubKey = (await this.getKeysArray()).find( + const keyRecord = (await this.getKeysArray()).find( (i) => i.basePath === account.basePath && i.pathIndex === account.pathIndex, - ).publicKey; + ); + assert(keyRecord, Errors.KeyringErrors.AddressDoesntExists); + const privKey = this.#privkeys[account.pathIndex.toString()]; + assert(privKey, Errors.KeyringErrors.AddressDoesntExists); keypair = { - privateKey: this.#privkeys[account.pathIndex.toString()], - publicKey: pubKey, + privateKey: privKey, + publicKey: keyRecord.publicKey, };🤖 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 413 - 421, The code assumes getKeysArray().find(...) returns a match before accessing .publicKey, which can be undefined and cause a runtime error; in the branch handling WalletType.privkey (inside the method using account), fetch the result of (await this.getKeysArray()).find(...) into a local variable (e.g., foundKey), check if foundKey is undefined, and if so handle it (throw a clear error, return a safe fallback, or log and abort) before using foundKey.publicKey when constructing keypair with this.#privkeys[account.pathIndex.toString()]; ensure the check references account.basePath and account.pathIndex so missing keys are handled deterministically.packages/extension/src/providers/ecash/ui/send-transaction/index.vue (2)
384-425:⚠️ Potential issue | 🟠 MajorGuard
fromAccountInfobefore buildingtxVerifyInfo.Line 386 can return
undefined, but Lines 408-409 and Line 425 dereference it unconditionally. If the selected account goes stale, the send flow throws instead of surfacing a recoverable error.🤖 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 384 - 425, The code builds txVerifyInfo using fromAccountInfo without checking it may be undefined; update sendAction to guard the result of keyring.getAccount(addressFrom.value) (fromAccountInfo) before constructing txVerifyInfo and before accessing fromAccountInfo.isHardware, fromAccountInfo.address, and fromAccountInfo.name — if fromAccountInfo is falsy, short-circuit the flow by showing a user-facing recoverable error (or return early) and avoid calling router.resolve with invalid data so the send flow does not throw.
52-56:⚠️ Potential issue | 🟠 MajorMake the fee picker drive local fee state.
The selector is rendered with hard-coded
REGULAR, andcurrentGasFee/gasPriceTypeare hard-coded the same way. Right now changing the fee tier cannot affect balance checks, max-send, or the verify payload.Also applies to: 188-190, 410-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 fee picker is currently hard-coded to GasPriceTypes.REGULAR so changes don't update local state used for balance/max-send/verify; update the <send-fee-select> usage to bind :selected-fee and :fee to the reactive locals (gasPriceType and currentGasFee or gasCostValues[gasPriceType]) and handle the component's emitted update event (e.g., `@change` or `@update`:selectedFee) to set gasPriceType and recalc currentGasFee accordingly; also replace the other hard-coded occurrences (lines referenced around currentGasFee/gasPriceType) so all fee-driven logic uses the bound reactive variables and triggers recalculation of balance checks, max-send, and verify payload when the picker changes.packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue (1)
270-273:⚠️ Potential issue | 🟡 MinorUse the normalized sender key in the failure path too.
The success branch stores activities under
displayFromAddress.value, but the catch block usestxData.value.fromAddress. WhenfromAddressis still the raw account identifier/public key, failed sends end up in a different history bucket.Minimal fix
txActivity.status = ActivityStatus.failed; activityState.addActivities([txActivity], { - address: txData.value.fromAddress, + address: displayFromAddress.value, network: network.value.name, });Also applies to: 286-289
🤖 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 - 273, The failure path stores the activity under the raw sender key (txData.value.fromAddress) instead of the normalized sender used on success, causing history fragmentation; update the catch/error branches to use the same normalized key (displayFromAddress.value) when calling activityState.addActivities (and any other addActivities calls around the verify/send flow, e.g., the block referenced at 286-289) so both success and failure insertions use displayFromAddress.value alongside network.value.name and the same txActivity/transactionHash payload.packages/extension/src/providers/ecash/libs/activity-handlers.ts (1)
47-79:⚠️ Potential issue | 🟠 MajorClassify sends before receives.
With a normal outgoing tx, the change output back to the wallet makes
isReceivetrue here, so Line 117 marks the activity as incoming and the value/address helpers run in receive mode. ComputeisSendfirst and only mark the tx as receive when 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; + });Also applies to: 114-117
🤖 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 47 - 79, The code currently computes isReceive before isSend, causing outgoing transactions with change outputs to be misclassified; compute isSend first by evaluating tx.inputs (using scriptToAddress and the same cashAddrPrefix) to set isSend, then set isReceive only when isSend is false and an owned output exists (use tx.outputs + scriptToAddress). Update subsequent uses of calculateTransactionValue and getTransactionAddresses to pass the computed isSend and the adjusted isReceive (i.e., isReceive = !isSend && hasOwnedOutput) so activity classification and value/address helpers behave correctly.
🧹 Nitpick comments (1)
packages/swap/src/providers/okx/index.ts (1)
485-486: Introduce typed interface for OKX v6 API parameters.Lines 485, 529, and 732 construct requests using v6-style params (
chainIndex,slippagePercent,swapMode), but use loose typing (Record<string, string>). Create a typed v6 params interface (e.g.,OKXSwapV6Params) and apply it to these three locations. Note: the existingOKXSwapParamsinterface uses different param names (chainId,slippage) and should not be conflated with v6 requirements.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swap/src/providers/okx/index.ts` around lines 485 - 486, Create a new interface OKXSwapV6Params that explicitly types the v6 parameter names (e.g., chainIndex: string, slippagePercent: string, swapMode: string, plus any other v6 fields used) instead of using Record<string,string>, then replace the loose-typed objects at the three v6 request sites with OKXSwapV6Params (the objects constructing chainIndex at the block with chainIndex: "501", the params built around line ~529, and the params around line ~732); ensure you do not change the existing OKXSwapParams interface (which uses chainId/slippage) and update the variable/argument type annotations where those v6 param objects are created or passed so the compiler enforces the correct v6 field names.
🤖 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`:
- Around line 26-38: The handler currently assumes message.params[0] is a valid
object which lets malformed inputs like { params: {} } or { params: [null] }
bypass the first guard and cause a throw; update the initial validation to
ensure message.params is an array with length > 0 and that message.params[0] is
a non-null object (e.g., Array.isArray(message.params) && message.params.length
> 0 && typeof message.params[0] === 'object' && message.params[0] !== null)
before casting to ECashSignParams, and then use safe property checks
(params?.toAddress, params?.amount, etc.) in the subsequent required-parameter
check so the handler returns the structured "ecash-sign: invalid params" or
"ecash-sign: missing required parameters" errors instead of throwing.
In `@packages/extension/src/providers/ecash/networks/ecash-base.ts`:
- Around line 138-145: The code dereferences marketData[0] (and similar later)
even though marketData is typed (CoinGeckoTokenMarket | null)[], so a null
element can bypass length checks; update the logic around
MarketData.getMarketData, marketData and currentPrice (and the later block with
the same pattern) to explicitly guard for null entries before accessing
properties: check that marketData[0] is not null (e.g., if (marketData[0] &&
marketData[0].current_price) ...) or find the first non-null element, then read
.current_price into currentPrice; ensure all places referencing marketData
elements perform the same null-checks to avoid throwing on nulls.
In `@packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts`:
- Around line 53-57: nonTokenUTXOs sorting/filtering uses extractSats() which
returns '0' for UTXOs that only have a value field, causing value-only inputs to
be treated as zero; update the logic in fee-calculator.ts (where nonTokenUTXOs
is built and the similar block around the other occurrence) to derive spendable
sats by checking both sats and value (e.g., prefer extractSats() but if it
returns '0' or undefined, parse the UTXO.value numeric string) before passing to
toBN(), so functions like the nonTokenUTXOs sort and any fee/insufficient-funds
calculations use the actual numeric value rather than defaulting to zero.
- Around line 124-141: The code currently only checks leftover computed against
txSizeWithChange, which misses the case where accumulated - amountSats can cover
a single-output tx (txSizeNoChange) but not a 2-output tx (txSizeWithChange);
compute both leftoverWithChange = accumulated - amountSats -
toBN(txSizeWithChange) and leftoverNoChange = accumulated - amountSats -
toBN(txSizeNoChange), then if leftoverWithChange < 546 sats but leftoverNoChange
>= 0 treat the transaction as a no-change spend: set txSize to txSizeNoChange,
set realFee to accumulated - amountSats (or Number(...) as used), set leftover
to "0" (or leftoverNoChange if you prefer explicit zero semantics for UI) and
return the fee values using existing helpers (fromBase, toBN) so the near-max
send path uses the 1-output fee instead of marking it dust.
In `@packages/extension/src/providers/ethereum/inject.ts`:
- Around line 71-80: The catch block around the sendMessageHandler call is
remapping every rejection to { code: 4001, message: e }, which hides original
provider error codes; update the logic in the method that calls
this.sendMessageHandler (the block handling this.name and request) to preserve
and rethrow the upstream error object if it already has { code, message } (i.e.,
return Promise.reject(e) or throw e), and only normalize/wrap errors that are
not provider-like (no numeric code or message) into a consistent shape; keep the
successful resp return behavior unchanged and mirror the eth_requestAccounts
handling that returns the upstream rejection unchanged.
In `@packages/swap/src/index.ts`:
- Around line 127-137: The fetch/JSON parsing for setting this.rwaFilterList
when this.walletId === WalletIdentifier.enkrypt can throw and reject init();
wrap the entire remote-check-and-fetch sequence in a try/catch (or add .catch
handlers) so any network, non-OK response, or JSON parse error is caught and
treated as non-fatal, assigning an empty array to rwaFilterList on error; update
the logic around the existing fetch(RWA_FILTER_LIST) and the outer
fetch(IP_COMPLY_URL) so failures return [] and do not rethrow, leaving init() to
continue.
- Around line 139-147: The RWA filter compares raw address strings and can miss
matches due to checksum/casing differences; update the filtering in
packages/swap/src/index.ts (the three filter calls on this.tokenList.all,
this.tokenList.top, this.tokenList.trending) to normalize addresses before
comparison: create a normalized set from this.rwaFilterList (e.g., all
lowercased) and compare each token's address normalized (e.g.,
t.address.toLowerCase()) against that set so casing differences cannot bypass
the filter.
---
Outside diff comments:
In
`@packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-token-select.vue`:
- Around line 2-17: Replace the non-semantic anchor used as the click trigger
with a proper button element to restore keyboard accessibility: change the root
element of the SendTokenSelect component (the element with class
"send-token-select" and the click handler that calls
emit('update:toggleTokenSelect')) to a <button type="button">, keep the same
classes and children (send-token-select__image, send-token-select__info,
send-token-select__arrow, <switch-arrow/>) and ensure any styling or
role-dependent CSS still applies; verify the click handler and any event emit
logic remains identical so update:toggleTokenSelect continues to fire.
In `@packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue`:
- Around line 310-317: In fetchAssets, ensure we always clear the loading flag
and provide a fallback asset on error: wrap
props.network.getAllTokens(addressFrom.value) with .catch and .finally, set
accountAssets.value = [] and selectedAsset.value = loadingAsset in the .catch
handler (or if allAssets is empty), and move isLoadingAssets.value = false into
.finally so the UI won't stay blocked; keep existing success path assigning
accountAssets.value and selectedAsset.value when tokens are returned.
In `@packages/extension/src/providers/solana/ui/send-transaction/index.vue`:
- Around line 455-474: The fetchAssets function sets isLoadingAssets true but
only clears it on the successful promise path; update fetchAssets (the promise
returned from props.network.getAllTokens inside fetchAssets) to handle errors
and always clear isLoadingAssets (use .catch or .finally) so the UI never stays
stuck, and on failure reset accountAssets and selectedAsset to fallback (e.g.,
loadingAsset) to avoid stale state; additionally, before assigning selectedAsset
= allAssets[0] or using .find against tokenParamData.contract, guard that
allAssets is non-empty/defined to avoid undefined assignments.
---
Duplicate comments:
In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts`:
- Around line 47-79: The code currently computes isReceive before isSend,
causing outgoing transactions with change outputs to be misclassified; compute
isSend first by evaluating tx.inputs (using scriptToAddress and the same
cashAddrPrefix) to set isSend, then set isReceive only when isSend is false and
an owned output exists (use tx.outputs + scriptToAddress). Update subsequent
uses of calculateTransactionValue and getTransactionAddresses to pass the
computed isSend and the adjusted isReceive (i.e., isReceive = !isSend &&
hasOwnedOutput) so activity classification and value/address helpers behave
correctly.
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 384-425: The code builds txVerifyInfo using fromAccountInfo
without checking it may be undefined; update sendAction to guard the result of
keyring.getAccount(addressFrom.value) (fromAccountInfo) before constructing
txVerifyInfo and before accessing fromAccountInfo.isHardware,
fromAccountInfo.address, and fromAccountInfo.name — if fromAccountInfo is falsy,
short-circuit the flow by showing a user-facing recoverable error (or return
early) and avoid calling router.resolve with invalid data so the send flow does
not throw.
- Around line 52-56: The fee picker is currently hard-coded to
GasPriceTypes.REGULAR so changes don't update local state used for
balance/max-send/verify; update the <send-fee-select> usage to bind
:selected-fee and :fee to the reactive locals (gasPriceType and currentGasFee or
gasCostValues[gasPriceType]) and handle the component's emitted update event
(e.g., `@change` or `@update`:selectedFee) to set gasPriceType and recalc
currentGasFee accordingly; also replace the other hard-coded occurrences (lines
referenced around currentGasFee/gasPriceType) so all fee-driven logic uses the
bound reactive variables and triggers recalculation of balance checks, max-send,
and verify payload when the picker changes.
In
`@packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue`:
- Around line 270-273: The failure path stores the activity under the raw sender
key (txData.value.fromAddress) instead of the normalized sender used on success,
causing history fragmentation; update the catch/error branches to use the same
normalized key (displayFromAddress.value) when calling
activityState.addActivities (and any other addActivities calls around the
verify/send flow, e.g., the block referenced at 286-289) so both success and
failure insertions use displayFromAddress.value alongside network.value.name and
the same txActivity/transactionHash payload.
In `@packages/keyring/src/index.ts`:
- Around line 413-421: The code assumes getKeysArray().find(...) returns a match
before accessing .publicKey, which can be undefined and cause a runtime error;
in the branch handling WalletType.privkey (inside the method using account),
fetch the result of (await this.getKeysArray()).find(...) into a local variable
(e.g., foundKey), check if foundKey is undefined, and if so handle it (throw a
clear error, return a safe fallback, or log and abort) before using
foundKey.publicKey when constructing keypair with
this.#privkeys[account.pathIndex.toString()]; ensure the check references
account.basePath and account.pathIndex so missing keys are handled
deterministically.
---
Nitpick comments:
In `@packages/swap/src/providers/okx/index.ts`:
- Around line 485-486: Create a new interface OKXSwapV6Params that explicitly
types the v6 parameter names (e.g., chainIndex: string, slippagePercent: string,
swapMode: string, plus any other v6 fields used) instead of using
Record<string,string>, then replace the loose-typed objects at the three v6
request sites with OKXSwapV6Params (the objects constructing chainIndex at the
block with chainIndex: "501", the params built around line ~529, and the params
around line ~732); ensure you do not change the existing OKXSwapParams interface
(which uses chainId/slippage) and update the variable/argument type annotations
where those v6 param objects are created or passed so the compiler enforces the
correct v6 field names.
🪄 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: ef99b9b7-935f-4857-8c9e-be4b8ffd5b3b
⛔ Files ignored due to path filters (4)
.yarn/versions/81e941ed.ymlis excluded by!**/.yarn/**.yarn/versions/8cbbf8cb.ymlis excluded by!**/.yarn/**packages/extension/src/providers/ecash/networks/icons/ecash.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (88)
.yarnrc.ymlpackage.jsonpackages/extension-bridge/package.jsonpackages/extension/env.d.tspackages/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/dapp-list/index.tspackages/extension/src/libs/keyring/keyring.tspackages/extension/src/libs/utils/initialize-wallet.tspackages/extension/src/libs/utils/networks.tspackages/extension/src/providers/bitcoin/ui/send-transaction/components/send-address-input.vuepackages/extension/src/providers/bitcoin/ui/send-transaction/components/send-alert.vuepackages/extension/src/providers/bitcoin/ui/send-transaction/components/send-token-select.vuepackages/extension/src/providers/bitcoin/ui/send-transaction/index.vuepackages/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/providers/ethereum/inject.tspackages/extension/src/providers/ethereum/libs/activity-handlers/index.tspackages/extension/src/providers/ethereum/libs/activity-handlers/providers/godwoken/configs.tspackages/extension/src/providers/ethereum/libs/activity-handlers/providers/godwoken/index.tspackages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.tspackages/extension/src/providers/ethereum/libs/assets-handlers/token-lists.tspackages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.tspackages/extension/src/providers/ethereum/networks/5ire.tspackages/extension/src/providers/ethereum/networks/godwoken.tspackages/extension/src/providers/ethereum/networks/index.tspackages/extension/src/providers/ethereum/tests/ethereum.events.test.tspackages/extension/src/providers/ethereum/tests/ethereum.requests.test.tspackages/extension/src/providers/polkadot/networks/assethub/assethub-dot.tspackages/extension/src/providers/polkadot/networks/assethub/assethub-ksm.tspackages/extension/src/providers/polkadot/networks/index.tspackages/extension/src/providers/polkadot/networks/unique/quartz.tspackages/extension/src/providers/solana/ui/send-transaction/components/send-address-input.vuepackages/extension/src/providers/solana/ui/send-transaction/components/send-alert.vuepackages/extension/src/providers/solana/ui/send-transaction/components/send-fee-select.vuepackages/extension/src/providers/solana/ui/send-transaction/components/send-token-select.vuepackages/extension/src/providers/solana/ui/send-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/hw-wallets/package.jsonpackages/hw-wallets/src/trezor/trezorConnect.tspackages/keyring/package.jsonpackages/keyring/src/index.tspackages/keyring/tests/generate.test.tspackages/name-resolution/package.jsonpackages/name-resolution/tests/ens.test.tspackages/name-resolution/tests/resolver.test.tspackages/request/package.jsonpackages/signers/bitcoin/package.jsonpackages/signers/bitcoin/tests/sign.test.tspackages/signers/ethereum/package.jsonpackages/signers/kadena/package.jsonpackages/signers/massa/package.jsonpackages/signers/polkadot/package.jsonpackages/storage/package.jsonpackages/swap/package.jsonpackages/swap/src/configs.tspackages/swap/src/index.tspackages/swap/src/providers/okx/index.tspackages/types/package.jsonpackages/types/src/index.tspackages/types/src/networks.tspackages/utils/package.json
💤 Files with no reviewable changes (12)
- packages/extension/src/providers/polkadot/networks/index.ts
- packages/extension/src/providers/ethereum/networks/5ire.ts
- packages/extension/src/providers/ethereum/libs/assets-handlers/types/tokenbalance-mew.ts
- packages/extension/src/providers/ethereum/networks/godwoken.ts
- packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts
- packages/extension/src/providers/ethereum/libs/activity-handlers/providers/godwoken/configs.ts
- packages/extension/src/libs/dapp-list/index.ts
- packages/extension/src/providers/ethereum/libs/activity-handlers/index.ts
- packages/extension/src/providers/polkadot/networks/unique/quartz.ts
- packages/extension/src/providers/ethereum/libs/assets-handlers/token-lists.ts
- packages/extension/src/providers/ethereum/libs/activity-handlers/providers/godwoken/index.ts
- packages/extension/src/providers/ethereum/networks/index.ts
✅ Files skipped from review due to trivial changes (35)
- .yarnrc.yml
- packages/extension/env.d.ts
- packages/signers/bitcoin/tests/sign.test.ts
- packages/extension/src/ui/action/views/verify-transaction/index.vue
- packages/extension/src/ui/action/views/deposit/index.vue
- packages/extension/src/providers/ethereum/tests/ethereum.events.test.ts
- packages/extension/src/libs/background/internal/index.ts
- packages/extension/src/providers/polkadot/networks/assethub/assethub-ksm.ts
- packages/name-resolution/tests/ens.test.ts
- packages/extension/src/providers/ecash/networks/index.ts
- packages/extension/src/ui/action/views/send-transaction/index.vue
- packages/extension/src/providers/ecash/tests/ecash.address.derivation.test.ts
- package.json
- packages/name-resolution/tests/resolver.test.ts
- packages/extension/src/providers/polkadot/networks/assethub/assethub-dot.ts
- packages/extension/src/types/messenger.ts
- packages/keyring/package.json
- packages/keyring/tests/generate.test.ts
- packages/utils/package.json
- packages/name-resolution/package.json
- packages/signers/polkadot/package.json
- packages/signers/bitcoin/package.json
- packages/extension-bridge/package.json
- packages/storage/package.json
- packages/signers/ethereum/package.json
- packages/signers/kadena/package.json
- packages/types/package.json
- packages/signers/massa/package.json
- packages/extension/src/providers/solana/ui/send-transaction/components/send-token-select.vue
- packages/extension/src/providers/ecash/types/ecash-chronik.ts
- packages/request/package.json
- packages/extension/src/libs/background/internal/ecash-sign.test.ts
- packages/swap/package.json
- packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue
- packages/extension/src/providers/ecash/libs/api-chronik.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/extension/src/ui/action/views/network-activity/index.vue
- packages/types/src/index.ts
- packages/extension/src/libs/utils/initialize-wallet.ts
- packages/extension/src/providers/ecash/networks/ecash-testnet.ts
- packages/extension/src/providers/ecash/types/ecash-token.ts
- packages/extension/src/libs/utils/networks.ts
- packages/extension/src/libs/activity-state/wrap-activity-handler.ts
- packages/extension/src/ui/action/utils/filters.ts
- packages/extension/src/types/base-network.ts
- packages/extension/src/providers/ecash/ui/send-transaction/components/send-alert.vue
…and UI components
Amatack
force-pushed
the
feature/ecash-integration
branch
from
eefeb23 to
fb63454
Compare
Author
|
Some of the issues reported by the bot seem to come from unrelated parts of the codebase (e.g. Ethereum provider) and not from this PR. |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/extension/src/providers/ecash/ui/send-transaction/index.vue (2)
246-255:⚠️ Potential issue | 🟡 Minor
parseInt()treats fractional amounts as zero.
parseInt('0.01')returns0, so valid sub-unit sends display the generic "Send" label instead of "Send 0.01 XEC". Use a decimal-safe comparison.🩹 Proposed fix
const sendButtonTitle = computed(() => { let title = 'Send'; - if (parseInt(sendAmount.value) > 0) + if (new BigNumber(sendAmount.value || '0').gt(0)) title = 'Send ' + formatFloatingPointValue(sendAmount.value).value + ' ' + selectedAsset.value?.symbol!.toUpperCase(); return title; });🤖 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 246 - 255, The computed sendButtonTitle uses parseInt(sendAmount.value) which treats fractional values like "0.01" as 0; replace the integer-only check with a decimal-safe numeric check (e.g., use Number(sendAmount.value) > 0 or parseFloat(sendAmount.value) > 0, or the existing formatFloatingPointValue/BigNumber helper if available) so that sendButtonTitle builds "Send {formattedAmount} {symbol}" for fractional amounts; update the conditional around sendAmount.value in the sendButtonTitle computed getter (referencing sendButtonTitle, sendAmount, formatFloatingPointValue, and selectedAsset) accordingly.
384-413:⚠️ Potential issue | 🟠 MajorGuard
getAccount()result before dereferencing.
PublicKeyRing.getAccount()can returnnullif the account is stale or missing. AccessingfromAccountInfo.address,.name, and.isHardwarewithout a null check will throw and break the send flow.🛡️ Proposed fix
const sendAction = async () => { const keyring = new PublicKeyRing(); const fromAccountInfo = await keyring.getAccount(addressFrom.value); + + if (!fromAccountInfo) { + console.error('[sendAction] Account not found for address:', addressFrom.value); + return; + } const txInfoData: any = { from: displayAddressFrom.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/index.vue` around lines 384 - 413, The call to PublicKeyRing.getAccount() inside sendAction can return null, so guard the result (fromAccountInfo) before dereferencing fields used to build txVerifyInfo; if fromAccountInfo is null/undefined, abort the sendAction flow (e.g., show an error/toast and return) or fallback to a safe value, and only access fromAccountInfo.address, fromAccountInfo.name, and fromAccountInfo.isHardware after that null check; update the code that constructs txVerifyInfo to use the validated fromAccountInfo and ensure any downstream logic that relied on those properties handles the early return or fallback.packages/extension/src/providers/ecash/libs/activity-handlers.ts (2)
95-98:⚠️ Potential issue | 🟠 MajorAdd
pkscripttorawInfo.inputsas well.
rawInfo.outputsincludepkscript, but inputs drop it. That leaves the BTC-like payload incomplete for activity consumers that inspect input scripts.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 95 - 98, rawInfo.inputs currently maps input objects in the tx.inputs transformation (the mapping inside activity-handlers.ts that builds inputs: tx.inputs.map(...)) without including the input script, so add a pkscript field to each mapped input (e.g., pkscript: input.outputScript ?? '') to mirror rawInfo.outputs and provide the BTC-like payload consumers expect; update the mapping that produces inputs (the arrow function creating address and value) to also include pkscript and ensure its value comes from input.outputScript (or an empty string when absent).
47-79:⚠️ Potential issue | 🟠 MajorTreat outbound activity as send-first, and pass the full owned-address set.
This still misclassifies normal sends with change:
isReceivecan becometruebeforeisSend, and passing only[normalizedAddress]means HD change routed to another wallet-owned address is still counted as external. That flipsisIncoming, transfer value, and counterparty data for outbound txs.🤖 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 47 - 79, Compute outbound detection before inbound and pass the full set of owned addresses rather than a single normalizedAddress: determine isSend by checking tx.inputs first (using scriptToAddress and cashAddrPrefix), then set isReceive only if not isSend by checking tx.outputs; when calling calculateTransactionValue and getTransactionAddresses use the full owned-address array (not just [normalizedAddress]) and prefer the isSend flag as the primary indicator for outbound transactions so transfers with HD change are classified as sends with internal change rather than incoming.
🧹 Nitpick comments (1)
packages/extension/src/providers/ecash/ui/send-transaction/index.vue (1)
52-56: Fee tier selection is static but may be intentional for eCash.The
selected-feeprop is hardcoded toGasPriceTypes.REGULARand never updated from the child component. However, reviewingbuildGasCostValuesinfee-calculator.ts, all fee tiers (ECONOMY, REGULAR, FAST, FASTEST) return identical values for eCash. If this is intentional (single fee tier for eCash), consider either removing the fee selector component or adding a brief comment explaining eCash uses a flat fee structure.🤖 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 SendTransaction view currently hardcodes the send-fee-select prop selected-fee to GasPriceTypes.REGULAR and never updates it; since buildGasCostValues in fee-calculator.ts returns identical values for all GasPriceTypes for eCash, either remove the <send-fee-select> usage or add an explicit comment and UI change indicating eCash uses a flat fee (so selection is disabled/hidden) to avoid confusion. Locate the send-fee-select usage and gasCostValues references in the component, and then either (A) remove the component and any selected-fee prop logic, or (B) keep it but disable the selector and add a concise inline comment mentioning buildGasCostValues produces identical fees for eCash (referencing GasPriceTypes and buildGasCostValues) so reviewers/users understand the selection is intentionally static.
🤖 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/networks/ecash-base.ts`:
- Around line 216-228: ecashOptions is missing a required feeHandler so
ECashNetwork.feeHandler becomes undefined; update the object passed to
createECashNetworkOptions to include a feeHandler property (e.g., feeHandler:
feeHandlerForChronik or feeHandler: defaultECashFeeHandler) that calls your
existing fee-estimation logic (or implement a minimal fee estimator and
reference it) so the default ecash network instance always has a valid function;
edit ecashOptions in
packages/extension/src/providers/ecash/networks/ecash-base.ts and wire the
feeHandler symbol to the appropriate existing function (or add and reference a
new default handler) used by send/estimate paths.
---
Duplicate comments:
In `@packages/extension/src/providers/ecash/libs/activity-handlers.ts`:
- Around line 95-98: rawInfo.inputs currently maps input objects in the
tx.inputs transformation (the mapping inside activity-handlers.ts that builds
inputs: tx.inputs.map(...)) without including the input script, so add a
pkscript field to each mapped input (e.g., pkscript: input.outputScript ?? '')
to mirror rawInfo.outputs and provide the BTC-like payload consumers expect;
update the mapping that produces inputs (the arrow function creating address and
value) to also include pkscript and ensure its value comes from
input.outputScript (or an empty string when absent).
- Around line 47-79: Compute outbound detection before inbound and pass the full
set of owned addresses rather than a single normalizedAddress: determine isSend
by checking tx.inputs first (using scriptToAddress and cashAddrPrefix), then set
isReceive only if not isSend by checking tx.outputs; when calling
calculateTransactionValue and getTransactionAddresses use the full owned-address
array (not just [normalizedAddress]) and prefer the isSend flag as the primary
indicator for outbound transactions so transfers with HD change are classified
as sends with internal change rather than incoming.
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 246-255: The computed sendButtonTitle uses
parseInt(sendAmount.value) which treats fractional values like "0.01" as 0;
replace the integer-only check with a decimal-safe numeric check (e.g., use
Number(sendAmount.value) > 0 or parseFloat(sendAmount.value) > 0, or the
existing formatFloatingPointValue/BigNumber helper if available) so that
sendButtonTitle builds "Send {formattedAmount} {symbol}" for fractional amounts;
update the conditional around sendAmount.value in the sendButtonTitle computed
getter (referencing sendButtonTitle, sendAmount, formatFloatingPointValue, and
selectedAsset) accordingly.
- Around line 384-413: The call to PublicKeyRing.getAccount() inside sendAction
can return null, so guard the result (fromAccountInfo) before dereferencing
fields used to build txVerifyInfo; if fromAccountInfo is null/undefined, abort
the sendAction flow (e.g., show an error/toast and return) or fallback to a safe
value, and only access fromAccountInfo.address, fromAccountInfo.name, and
fromAccountInfo.isHardware after that null check; update the code that
constructs txVerifyInfo to use the validated fromAccountInfo and ensure any
downstream logic that relied on those properties handles the early return or
fallback.
---
Nitpick comments:
In `@packages/extension/src/providers/ecash/ui/send-transaction/index.vue`:
- Around line 52-56: The SendTransaction view currently hardcodes the
send-fee-select prop selected-fee to GasPriceTypes.REGULAR and never updates it;
since buildGasCostValues in fee-calculator.ts returns identical values for all
GasPriceTypes for eCash, either remove the <send-fee-select> usage or add an
explicit comment and UI change indicating eCash uses a flat fee (so selection is
disabled/hidden) to avoid confusion. Locate the send-fee-select usage and
gasCostValues references in the component, and then either (A) remove the
component and any selected-fee prop logic, or (B) keep it but disable the
selector and add a concise inline comment mentioning buildGasCostValues produces
identical fees for eCash (referencing GasPriceTypes and buildGasCostValues) so
reviewers/users understand the selection is intentionally static.
🪄 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: bb3e22b7-913d-4b8b-b370-d6f2ac360aca
⛔ 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
✅ Files skipped from review due to trivial changes (13)
- packages/extension/src/libs/background/internal/index.ts
- packages/extension/src/ui/action/views/send-transaction/index.vue
- packages/extension/src/types/messenger.ts
- packages/extension/src/providers/ecash/tests/ecash.address.derivation.test.ts
- packages/extension/src/providers/ecash/networks/index.ts
- packages/extension/src/providers/ecash/types/ecash-token.ts
- packages/keyring/tests/generate.test.ts
- packages/extension/src/providers/ecash/types/ecash-chronik.ts
- packages/extension/src/libs/background/internal/ecash-sign.test.ts
- packages/extension/src/providers/ecash/types/ecash-network.ts
- packages/extension/src/providers/ecash/tests/utils.test.ts
- packages/extension/src/providers/ecash/ui/libs/fee-calculator.ts
- packages/extension/src/providers/ecash/libs/api-chronik.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/extension/package.json
- packages/extension/src/libs/utils/initialize-wallet.ts
- packages/extension/src/ui/action/views/verify-transaction/index.vue
- packages/extension/src/ui/action/views/network-activity/index.vue
- packages/extension/src/libs/activity-state/wrap-activity-handler.ts
- packages/extension/src/ui/action/views/deposit/index.vue
- packages/extension/src/types/provider.ts
- packages/extension/src/providers/ecash/networks/ecash-testnet.ts
- packages/extension/src/providers/ecash/ui/send-transaction/components/send-address-input.vue
- packages/keyring/src/index.ts
- packages/extension/src/types/base-network.ts
- packages/extension/src/providers/ecash/ui/send-transaction/components/send-alert.vue
- packages/types/src/networks.ts
- packages/extension/src/libs/background/internal/ecash-sign.ts
- packages/extension/src/libs/utils/networks.ts
- packages/extension/src/providers/ecash/ui/send-transaction/verify-transaction/index.vue
Comment on lines
+216
to
+228
| const ecashOptions = createECashNetworkOptions({ | ||
| name: NetworkNames.ECash, | ||
| name_long: 'eCash', | ||
| homePage: 'https://e.cash/', | ||
| blockExplorerTX: 'https://explorer.e.cash/tx/[[txHash]]', | ||
| blockExplorerAddr: 'https://explorer.e.cash/address/ecash:[[address]]', | ||
| isTestNetwork: false, | ||
| currencyName: 'XEC', | ||
| currencyNameLong: 'eCash', | ||
| coingeckoID: 'ecash', | ||
| node: 'https://chronik-native1.fabien.cash', | ||
| dust: 546, | ||
| }); |
There was a problem hiding this comment.
Wire a feeHandler into the default eCash network config.
ECashNetwork.feeHandler is treated as required, but ecashOptions doesn't provide one. The default ecash instance will therefore carry undefined, so any send or fee-estimation path that calls network.feeHandler() will throw at runtime.
🤖 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
216 - 228, ecashOptions is missing a required feeHandler so
ECashNetwork.feeHandler becomes undefined; update the object passed to
createECashNetworkOptions to include a feeHandler property (e.g., feeHandler:
feeHandlerForChronik or feeHandler: defaultECashFeeHandler) that calls your
existing fee-estimation logic (or implement a minimal fee estimator and
reference it) so the default ecash network instance always has a valid function;
edit ecashOptions in
packages/extension/src/providers/ecash/networks/ecash-base.ts and wire the
feeHandler symbol to the appropriate existing function (or add and reference a
new default handler) used by send/estimate paths.
Author
|
Some of the issues reported by the bot seem to come from unrelated parts of the codebase (e.g. Ethereum provider) and not from this PR. |
Author
|
@kvhnuke could you please review this PR? |
|
@kvhnuke Can we have some eyes on this PR ? I would love to see this merged. |
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
2 participants
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
Release Notes