Skip to content
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

Namadillo: Fees #904

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions apps/namadillo/src/App/Common/FeeWarning.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { IoWarningOutline } from "react-icons/io5";

export const FeeWarning = (): JSX.Element => {
return (
<div className="flex text-xs items-center gap-1 text-yellow">
<i className="text-sm">
<IoWarningOutline />
</i>
Remember to keep a small amount of NAM for fees
</div>
);
};
38 changes: 26 additions & 12 deletions apps/namadillo/src/App/Common/TransactionFees.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,42 @@
import { gasLimitsAtom, minimumGasPriceAtom } from "atoms/fees";
import clsx from "clsx";
import { useGasEstimate } from "hooks/useGasEstimate";
import { useAtomValue } from "jotai";
import { TxKind } from "types";
import { FeeWarning } from "./FeeWarning";
import { NamCurrency } from "./NamCurrency";
import { TextLink } from "./TextLink";

type TransactionFeesProps = {
txKind: TxKind;
numberOfTransactions: number;
displayWarning?: boolean;
className?: string;
};

export const TransactionFees = ({
txKind,
numberOfTransactions,
displayWarning,
className,
}: TransactionFeesProps): JSX.Element => {
const { calculateMinGasRequired } = useGasEstimate();
const minimumGas = calculateMinGasRequired(numberOfTransactions);
const gasLimits = useAtomValue(gasLimitsAtom);
const gasPrice = useAtomValue(minimumGasPriceAtom);

if (!gasLimits.isSuccess || !gasPrice.isSuccess || numberOfTransactions === 0)
return <></>;

if (!minimumGas || minimumGas.eq(0)) return <></>;
return (
<div className={clsx("text-white text-sm", className)}>
<TextLink>Transaction fee:</TextLink>{" "}
<NamCurrency
className="font-medium"
amount={minimumGas}
forceBalanceDisplay={true}
/>
<div className={clsx("flex flex-col", className)}>
<div className="text-white text-sm">
Transaction fee:{" "}
<NamCurrency
className="font-medium"
forceBalanceDisplay={true}
amount={gasPrice.data.multipliedBy(
gasLimits.data[txKind].native.multipliedBy(numberOfTransactions)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this still be correct in cases where a reveal PK is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need to review it again after the batch tx PR gets in

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With batch Tx in, RevealPk wasn't considered when calculating fees - it is overestimating though so will work as is (though is incorrect). We still need to determine how fees can be calculated when we have a batch with of Txs, and (currently) at most 2 Tx types in that batch (RevealPK + the type of other Txs). I'm up for taking a look, and we don't necessarily need to address that in this PR!

)}
/>
</div>
{displayWarning && <FeeWarning />}
</div>
);
};
1 change: 1 addition & 0 deletions apps/namadillo/src/App/Governance/SubmitVote.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ export const WithProposalId: React.FC<{ proposalId: bigint }> = ({
</Stack>
<footer>
<TransactionFees
txKind="VoteProposal"
className="flex justify-between"
numberOfTransactions={1}
/>
Expand Down
14 changes: 10 additions & 4 deletions apps/namadillo/src/App/Staking/IncrementBonding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,16 @@ const IncrementBonding = (): JSX.Element => {
>
{isPerformingBond ? "Processing..." : errorMessage || "Stake"}
</ActionButton>
<TransactionFees
className="absolute right-4 top-1/2 -translate-y-1/2"
numberOfTransactions={Object.keys(updatedAmountByAddress).length}
/>
{gasLimits.isSuccess && (
<TransactionFees
txKind="Bond"
className="absolute text-right right-4 top-1/2 -translate-y-1/2"
displayWarning={true}
numberOfTransactions={
Object.keys(updatedAmountByAddress).length
}
/>
)}
</div>
</form>
</ModalContainer>
Expand Down
4 changes: 3 additions & 1 deletion apps/namadillo/src/App/Staking/ReDelegateAssignStake.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ export const ReDelegateAssignStake = ({
: "Re-Delegate"}
</ActionButton>
<TransactionFees
className="absolute right-4 top-1/2 -translate-y-1/2"
txKind="Redelegation"
displayWarning={true}
className="absolute text-right right-4 top-1/2 -translate-y-1/2"
numberOfTransactions={numberOfTransactions}
/>
</div>
Expand Down
4 changes: 3 additions & 1 deletion apps/namadillo/src/App/Staking/Unstake.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,9 @@ const Unstake = (): JSX.Element => {
: validationMessage || "Unstake"}
</ActionButton>
<TransactionFees
className="absolute right-4 top-1/2 -translate-y-1/2"
txKind="Unbond"
displayWarning={true}
className="absolute text-right right-4 top-1/2 -translate-y-1/2"
numberOfTransactions={Object.keys(updatedAmountByAddress).length}
/>
</div>
Expand Down
3 changes: 2 additions & 1 deletion apps/namadillo/src/atoms/fees/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export const fetchMinimumGasPrice = async (
({ token }) => token === nativeToken
);
invariant(!!nativeTokenCost, "Error querying minimum gas price");
const asBigNumber = new BigNumber(nativeTokenCost.amount);
// TODO: this should be removed after indexer error is fixed!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is fixed now.

const asBigNumber = new BigNumber(nativeTokenCost.amount).dividedBy(100000);
invariant(
!asBigNumber.isNaN(),
"Error converting minimum gas price to BigNumber"
Expand Down
Loading