feat: show network fee for send tx, create token and create nano dialogs#144
feat: show network fee for send tx, create token and create nano dialogs#144pedroferreira1 merged 2 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughA new NetworkFee component and formatHtrAmount helper are added and exported. The snap manifest shasum is updated. The NetworkFee UI is integrated into createNano, createToken (refactored to show fee model, network fee, deposit), and sendTransaction dialogs; a small helper getFeeModelLabel is added in createToken. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/snap/src/components/NetworkFee.tsx`:
- Around line 11-16: The helper formatHtrAmount currently treats 0n as missing
and returns '-', which hides legitimate zero fees; update formatHtrAmount so
only undefined (or null if applicable) returns '-' and 0n is formatted via
numberUtils.prettyValue and appended with
libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol (e.g., "0 SYMBOL"); locate
formatHtrAmount and adjust the conditional (remove the amount === 0n check) so
zero values render as "0 <symbol>" using the existing numberUtils.prettyValue
and symbol constant.
In `@packages/snap/src/dialogs/sendTransaction.tsx`:
- Line 125: The other dialogs are reading the wrong property for fees; in the
CreateToken dialog (component CreateToken, around where NetworkFee is rendered)
replace uses of data.networkFee with data.fee so the NetworkFee component
receives the backend-provided fee, and in the CreateNano dialog (component
CreateNano, where NetworkFee is rendered near the submit/summary) do the same
replacement of data.networkFee -> data.fee to prevent the fallback "-" from
showing; update any related prop names or typings if needed to match the fee
field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2235400-2dbb-4f55-96b9-d473ec3dc11e
📒 Files selected for processing (6)
packages/snap/snap.manifest.jsonpackages/snap/src/components/NetworkFee.tsxpackages/snap/src/components/index.tspackages/snap/src/dialogs/createNano.tsxpackages/snap/src/dialogs/createToken.tsxpackages/snap/src/dialogs/sendTransaction.tsx
| export const formatHtrAmount = (amount?: bigint): string => { | ||
| if (amount === undefined || amount === 0n) { | ||
| return '-'; | ||
| } | ||
| return `${numberUtils.prettyValue(amount)} ${libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol}`; | ||
| } |
There was a problem hiding this comment.
Render zero fees as 0, not -.
0n is still a real amount. Treating it as missing data makes zero-fee or zero-deposit confirmations ambiguous in every dialog that reuses this helper.
🛠 Proposed fix
export const formatHtrAmount = (amount?: bigint): string => {
- if (amount === undefined || amount === 0n) {
+ if (amount === undefined) {
return '-';
}
return `${numberUtils.prettyValue(amount)} ${libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const formatHtrAmount = (amount?: bigint): string => { | |
| if (amount === undefined || amount === 0n) { | |
| return '-'; | |
| } | |
| return `${numberUtils.prettyValue(amount)} ${libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol}`; | |
| } | |
| export const formatHtrAmount = (amount?: bigint): string => { | |
| if (amount === undefined) { | |
| return '-'; | |
| } | |
| return `${numberUtils.prettyValue(amount)} ${libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/snap/src/components/NetworkFee.tsx` around lines 11 - 16, The helper
formatHtrAmount currently treats 0n as missing and returns '-', which hides
legitimate zero fees; update formatHtrAmount so only undefined (or null if
applicable) returns '-' and 0n is formatted via numberUtils.prettyValue and
appended with libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol (e.g., "0
SYMBOL"); locate formatHtrAmount and adjust the conditional (remove the amount
=== 0n check) so zero values render as "0 <symbol>" using the existing
numberUtils.prettyValue and symbol constant.
There was a problem hiding this comment.
@pedroferreira1 I was about to make the same comment as the rabbit: shouldn't we hide the "Network fee" entirely when it is zero? Or maybe show explicitly zero instead of -?
Not a blocking comment, as this was probably discussed in a design somewhere before, but it feels weird to me as it is now.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
As we discussed, this is the UI defined for the screen on figma. I actually liked it, to be honest
| <Divider /> | ||
| {renderOutputs(params.outputs, data.tokenDetails)} | ||
| {renderChangeAddress(params.changeAddress)} | ||
| <NetworkFee networkFee={data.fee} showDivider /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Dialog consumers:"
rg -n -C2 '\bdata\.(fee|networkFee|depositAmount)\b' packages/snap/src/dialogs
echo
echo "Potential producers / assignments:"
rg -n -C3 '\b(networkFee|fee|depositAmount)\s*:' packages -g '!**/dist/**' -g '!**/node_modules/**'Repository: HathorNetwork/hathor-rpc-lib
Length of output: 21306
🏁 Script executed:
# Find type definitions for different prompt types
rg -n 'CreateTokenConfirmationPrompt|SendNanoContractTxConfirmationPrompt|SendTransactionConfirmationPrompt' packages/hathor-rpc-handler/src/types --type ts
# Look for where networkFee and depositAmount are assigned
rg -n -B3 -A3 'networkFee\s*=' packages/hathor-rpc-handler/src -g '!**/*test.ts'
# Check if there's a field mapping or transform in the handler files
rg -n 'depositAmount|networkFee' packages/hathor-rpc-handler/src/rpcMethods -g '!**/*test.ts'Repository: HathorNetwork/hathor-rpc-lib
Length of output: 1303
🏁 Script executed:
# Read the type definitions to see what fields they have
sed -n '223,250p' packages/hathor-rpc-handler/src/types/prompt.ts
sed -n '228,250p' packages/hathor-rpc-handler/src/types/prompt.ts
sed -n '312,330p' packages/hathor-rpc-handler/src/types/prompt.tsRepository: HathorNetwork/hathor-rpc-lib
Length of output: 2146
🏁 Script executed:
# Find CreateTokenParams and NanoContractParams definitions
rg -n 'type CreateTokenParams|interface CreateTokenParams' packages/hathor-rpc-handler/src/types -A15
rg -n 'type NanoContractParams|interface NanoContractParams' packages/hathor-rpc-handler/src/types -A15Repository: HathorNetwork/hathor-rpc-lib
Length of output: 2608
This dialog is correct; the bugs are in the other dialogs.
sendTransaction.tsx correctly reads data.fee matching the actual type definition. The issues are in packages/snap/src/dialogs/createToken.tsx (lines 44-45) and packages/snap/src/dialogs/createNano.tsx (line 190), which incorrectly read data.networkFee when the backend provides fee. Both dialogs will display - (fallback value) instead of the actual fee.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/snap/src/dialogs/sendTransaction.tsx` at line 125, The other dialogs
are reading the wrong property for fees; in the CreateToken dialog (component
CreateToken, around where NetworkFee is rendered) replace uses of
data.networkFee with data.fee so the NetworkFee component receives the
backend-provided fee, and in the CreateNano dialog (component CreateNano, where
NetworkFee is rendered near the submit/summary) do the same replacement of
data.networkFee -> data.fee to prevent the fallback "-" from showing; update any
related prop names or typings if needed to match the fee field.
pedroferreira1
force-pushed
the
feat/snap-network-fee
branch
from
1329b0c to
a0399a7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/snap/src/dialogs/createToken.tsx (1)
62-65: Confirm intentional formatting difference for "Amount" vs fee fields.The "Network fee" and "Deposit amount" cards use
formatHtrAmount()which appends the HTR symbol, while the "Amount" card usesnumberUtils.prettyValue(amount)without a symbol. This is likely intentional since "Amount" refers to the quantity of the new token being created (not HTR), but verify this matches the desired UX for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/snap/src/dialogs/createToken.tsx` around lines 62 - 65, Verify whether the "Amount" card should display a unit/symbol like the HTR fields: inspect the Card usages for networkFee/depositAmount (which call formatHtrAmount) and the "Amount" field (which uses numberUtils.prettyValue(amount)); if the intent is to show the new token quantity without HTR, make the UX explicit by either appending the token symbol to the description (e.g., prettyValue(amount) + " TOKEN_SYMBOL") or changing the title to "Amount (tokens)"; otherwise document the intentional difference and keep numberUtils.prettyValue(amount) as-is.
🤖 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/snap/src/dialogs/createToken.tsx`:
- Around line 62-65: Verify whether the "Amount" card should display a
unit/symbol like the HTR fields: inspect the Card usages for
networkFee/depositAmount (which call formatHtrAmount) and the "Amount" field
(which uses numberUtils.prettyValue(amount)); if the intent is to show the new
token quantity without HTR, make the UX explicit by either appending the token
symbol to the description (e.g., prettyValue(amount) + " TOKEN_SYMBOL") or
changing the title to "Amount (tokens)"; otherwise document the intentional
difference and keep numberUtils.prettyValue(amount) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f3f669f-2f92-424b-89e6-5e352a2722a7
📒 Files selected for processing (6)
packages/snap/snap.manifest.jsonpackages/snap/src/components/NetworkFee.tsxpackages/snap/src/components/index.tspackages/snap/src/dialogs/createNano.tsxpackages/snap/src/dialogs/createToken.tsxpackages/snap/src/dialogs/sendTransaction.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/snap/src/dialogs/sendTransaction.tsx
- packages/snap/src/components/NetworkFee.tsx
- packages/snap/snap.manifest.json
- packages/snap/src/dialogs/createNano.tsx
…alog for the new token version parameter
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/snap/src/dialogs/createToken.tsx`:
- Around line 28-35: The helper getFeeModelLabel currently treats any unknown
version as 'Deposit Token'; update it so only undefined maps to 'Deposit Token'
and any other unknown string is surfaced explicitly (e.g., return 'Unsupported
Token Version' or throw an error) instead of falling through. Locate
getFeeModelLabel and change the switch/logic to: case 'fee' -> 'Fee Token'; case
'deposit' or undefined -> 'Deposit Token'; default -> return a clear
unsupported-version marker (or throw) so unexpected params.version values are
not silently mapped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bbd51416-247b-48be-9b0c-d4380ab167a7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
packages/snap/package.jsonpackages/snap/snap.manifest.jsonpackages/snap/src/dialogs/createNano.tsxpackages/snap/src/dialogs/createToken.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/snap/src/dialogs/createNano.tsx
- packages/snap/snap.manifest.json
pedroferreira1
requested review from
tuliomir
and removed request for
andreabadesso
| export const formatHtrAmount = (amount?: bigint): string => { | ||
| if (amount === undefined || amount === 0n) { | ||
| return '-'; | ||
| } | ||
| return `${numberUtils.prettyValue(amount)} ${libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol}`; | ||
| } |
There was a problem hiding this comment.
@pedroferreira1 I was about to make the same comment as the rabbit: shouldn't we hide the "Network fee" entirely when it is zero? Or maybe show explicitly zero instead of -?
Not a blocking comment, as this was probably discussed in a design somewhere before, but it feels weird to me as it is now.
github-project-automation
bot
moved this from In Progress (WIP)
to In Review (WIP)
in Hathor Network
github-project-automation
bot
moved this from In Review (WIP)
to Waiting to be deployed
in Hathor Network
3 participants
Motivation
With the addition of Fee based tokens, we now have network fee for sending transactions in the network. We must show the fee for the user approval.
Acceptance Criteria
Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets mergedSummary by CodeRabbit