-
Notifications
You must be signed in to change notification settings - Fork 2
feat: show network fee for send tx, create token and create nano dialogs #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /** | ||
| * Copyright (c) Hathor Labs and its affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { Bold, Box, Divider, Text } from '@metamask/snaps-sdk/jsx'; | ||
| import { constants as libConstants, numberUtils } from '@hathor/wallet-lib'; | ||
|
|
||
| export const formatHtrAmount = (amount?: bigint): string => { | ||
| if (amount === undefined || amount === 0n) { | ||
| return '-'; | ||
| } | ||
| return `${numberUtils.prettyValue(amount)} ${libConstants.DEFAULT_NATIVE_TOKEN_CONFIG.symbol}`; | ||
| } | ||
|
|
||
| export const NetworkFee = ({ networkFee, showDivider = false }: { networkFee?: bigint; showDivider?: boolean }) => { | ||
| return ( | ||
| <Box> | ||
| {showDivider ? <Divider /> : null} | ||
| <Text><Bold>Network fee:</Bold> {formatHtrAmount(networkFee)}</Text> | ||
| </Box> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,3 +6,4 @@ | |
| */ | ||
|
|
||
| export { PushTxWarning } from './PushTxWarning'; | ||
| export { NetworkFee, formatHtrAmount } from './NetworkFee'; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ | |
| import { REQUEST_METHODS, DIALOG_TYPES } from '../constants'; | ||
| import { Bold, Box, Container, Divider, Heading, Icon, Section, Text, Tooltip } from '@metamask/snaps-sdk/jsx'; | ||
| import { constants as libConstants, numberUtils, helpersUtils } from '@hathor/wallet-lib'; | ||
| import { PushTxWarning } from '../components'; | ||
| import { NetworkFee, PushTxWarning } from '../components'; | ||
|
|
||
| const renderInputs = (inputs) => { | ||
| if (!inputs) { | ||
|
|
@@ -122,11 +122,12 @@ export const sendTransactionPage = async (data, params, origin) => ( | |
| <Divider /> | ||
| {renderOutputs(params.outputs, data.tokenDetails)} | ||
| {renderChangeAddress(params.changeAddress)} | ||
| <NetworkFee networkFee={data.fee} showDivider /> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 🤖 Prompt for AI Agents |
||
| </Section> | ||
| <PushTxWarning pushTx={data.pushTx} /> | ||
| </Box> | ||
| </Container> | ||
| ), | ||
| }, | ||
| }) | ||
| ) | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Render zero fees as
0, not-.0nis 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
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, this is the UI defined for the screen on figma. I actually liked it, to be honest