Repository navigation
feat(tron): estimate send fees conservatively in onAmountInput - #554
ulissesferreira wants to merge 1 commit into
Conversation
a07d39c to
18d5846
Compare
ulissesferreira
changed the title
onAmountInput
d2514de to
d8db631
Compare
d8db631 to
c5c45f8
Compare
mikesposito
left a comment
mikesposito
left a comment
There was a problem hiding this comment.
Looks good! just a couple of small suggestions on the changelog
| const baseTrxFee = new BigNumber( | ||
| fees.find((fee) => fee.asset.type === nativeTokenId)?.asset.amount ?? | ||
| '0', | ||
| ); |
There was a problem hiding this comment.
nit (as this was already the case): this falls back to 0 silently in case there's no native token fee, which is unexpected: perhaps we should throw an error in these cases?
There was a problem hiding this comment.
What error should we throw?
There was a problem hiding this comment.
Actually, this is aligned with what some other methods are doing. So I would suggest if we add an error we do it in another PR, does that sound good?
82f6570 to
476d65f
Compare
476d65f to
5366887
Compare
|
There was a problem hiding this comment.
🟡 Changes recommended
Amount validation mishandles empty recipients, can double-count activation fees, and omits activation reserves for TRC-10 transfers, causing incorrect rejections or acceptance of unaffordable sends.
3 open issues
What changed in this PR
This PR adds fee checks to the Tron snap’s amount validation before a recipient is entered, preparing for getMaxSendableAmount.
Changes:
- Estimates fees using a zero-address probe when the destination is unknown, with an additional activation reserve for native TRX.
- Extracts reusable account-activation fee calculation and adds validation tests.
- Documents the behavior change and updates the snap bundle checksum.
| File | Description |
|---|---|
packages/tron-wallet-snap/src/services/send/types.ts |
Adds the activation-fee result type. |
packages/tron-wallet-snap/src/services/send/FeeCalculatorService.ts |
Extracts and exposes activation-fee calculation. |
packages/tron-wallet-snap/src/services/send/FeeCalculatorService.test.ts |
Tests activation costs, resources, and unavailable chain parameters. |
packages/tron-wallet-snap/src/handlers/clientRequest/clientRequest.ts |
Adds unknown-recipient fee estimation and native TRX activation reserves. |
packages/tron-wallet-snap/src/handlers/clientRequest/clientRequest.test.ts |
Tests fee validation without a recipient for TRX and TRC-20. |
packages/tron-wallet-snap/snap.manifest.json |
Updates the bundle checksum. |
packages/tron-wallet-snap/CHANGELOG.md |
Documents the amount-validation change. |
Review effort: Balanced
| const sendTransaction = await this.#sendService.buildTransaction({ | ||
| fromAccountId: accountId, | ||
| toAddress, | ||
| toAddress: toAddress ?? NULL_ADDRESS, |
There was a problem hiding this comment.
To check this one with a null address
| * sender's staked Bandwidth cannot cover the account creation. | ||
| */ | ||
| const activationTrxFee = | ||
| !toAddress && isNativeToken |
| assetId === nativeTokenId ? valueBN.plus(trxFee) : trxFee; | ||
| const totalTrxToSpend = (isNativeToken ? valueBN : ZERO) | ||
| .plus(baseTrxFee) | ||
| .plus(activationTrxFee); |
There was a problem hiding this comment.
ok to check this double calculation in that case
Explanation
onAmountInputreturned valid without checking fees whentoAddresswas absent. It now builds a probe transaction to the Tron zero address (T9yD14Nj9j7xAB4dbGeiX9h8unkKHxuWwb) and estimates fees, adding the account activation reserve for native TRX. The zero address is used because Tron rejects self-transfers, so the sender's own address cannot serve as the probe. TRC-20 transfers estimate the transfer fee without an activation reserve. This makes typing-time validation consistent with the destination-known case and preparesgetMaxSendableAmount.References
Ticket: WPN-2375
Checklist