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

fix: issue with fee UI react rendering loop #65

Closed
wants to merge 1 commit 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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ name: ci
on:
pull_request:
branches:
- '**'
- "**"

jobs:
lint_test:
uses: babylonlabs-io/.github/.github/workflows/[email protected]
with:
run-build: true
run-unit-tests: true
run-unit-tests: true
8 changes: 4 additions & 4 deletions .github/workflows/publish.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ name: docker_publish
on:
push:
branches:
- 'main'
- 'dev'
- "main"
- "dev"
tags:
- '*'
- "*"

jobs:
lint_test:
uses: babylonlabs-io/.github/.github/workflows/[email protected]
with:
run-build: true
run-unit-tests: true

docker_build:
needs: [lint_test]
runs-on: ubuntu-22.04
Expand Down
49 changes: 36 additions & 13 deletions src/app/components/Staking/Form/StakingAmount.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ interface StakingAmountProps {
btcWalletBalanceSat?: number;
onStakingAmountSatChange: (inputAmountSat: number) => void;
reset: boolean;
isSufficientBalance: boolean;
}

export const StakingAmount: React.FC<StakingAmountProps> = ({
Expand All @@ -20,11 +21,13 @@ export const StakingAmount: React.FC<StakingAmountProps> = ({
btcWalletBalanceSat,
onStakingAmountSatChange,
reset,
isSufficientBalance,
}) => {
const [value, setValue] = useState("");
const [error, setError] = useState("");
// Track if the input field has been interacted with
const [touched, setTouched] = useState(false);
const [blurred, setBlurred] = useState(false);

const errorLabel = "Staking amount";
const generalErrorMessage = "You should input staking amount";
Expand All @@ -51,16 +54,8 @@ export const StakingAmount: React.FC<StakingAmountProps> = ({
}
};

const handleBlur = (_e: FocusEvent<HTMLInputElement>) => {
if (!btcWalletBalanceSat) return;
setTouched(true);

if (value === "") {
onStakingAmountSatChange(0);
setError(generalErrorMessage);
return;
}

useEffect(() => {
if (!btcWalletBalanceSat || value === "") return;
const numValue = parseFloat(value);
const satoshis = btcToSatoshi(numValue);

Expand All @@ -71,7 +66,7 @@ export const StakingAmount: React.FC<StakingAmountProps> = ({
message: `${errorLabel} must be a valid number.`,
},
{
valid: numValue !== 0,
valid: numValue > 0,
message: `${errorLabel} must be greater than 0.`,
},
{
Expand All @@ -91,18 +86,46 @@ export const StakingAmount: React.FC<StakingAmountProps> = ({
message: `${errorLabel} must have no more than 8 decimal points.`,
},
];

// Find the first failing validation
const firstInvalid = validations.find((validation) => !validation.valid);

if (firstInvalid) {
onStakingAmountSatChange(0);
setError(firstInvalid.message);
return;
} else {
setError("");
onStakingAmountSatChange(satoshis);
setValue(maxDecimals(satoshiToBtc(satoshis), 8).toString());
}
// The fee + amount check can only be placed after onStakingAmountSatChange is called
// This is due to isSufficientBalance being dependent on the value of
// stakingAmountSat in parent component
if (!isSufficientBalance && validateDecimalPoints(value)) {
return setError(
"Not enough balance to cover the staking value and its fees",
);
}
}, [
isSufficientBalance,
blurred,
value,
btcWalletBalanceSat,
minStakingAmountSat,
maxStakingAmountSat,
onStakingAmountSatChange,
coinName,
]);

const handleBlur = (_e: FocusEvent<HTMLInputElement>) => {
if (!btcWalletBalanceSat) return;
setTouched(true);

if (value === "") {
onStakingAmountSatChange(0);
setError(generalErrorMessage);
return;
}
setBlurred(!blurred);
};

const minStakeAmount = maxDecimals(satoshiToBtc(minStakingAmountSat), 8);
Expand Down
23 changes: 12 additions & 11 deletions src/app/components/Staking/Form/StakingFee.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { LoadingSmall } from "../../Loading/Loading";
interface StakingFeeProps {
stakingFeeSat: number;
selectedFeeRate: number;
onSelectedFeeRateChange: (fee: number) => void;
onSelectedFeeRateChange: (feeRate: number) => void;
reset: boolean;
// optional as component shows loading state
mempoolFeeRates?: Fees;
Expand Down Expand Up @@ -46,6 +46,11 @@ export const StakingFee: React.FC<StakingFeeProps> = ({
}
};

// If staking fee has not been calculated, return null
Copy link
Member

Choose a reason for hiding this comment

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

Why are we doing this instead of what we had before? Seems that we have a different behavior for mempool fee rates

if (!stakingFeeSat) {
return null;
}

const defaultModeRender = () => {
return (
<div className="flex flex-col justify-center gap-1 items-center">
Expand All @@ -57,16 +62,12 @@ export const StakingFee: React.FC<StakingFeeProps> = ({
) : (
<LoadingSmall text="Loading recommended fee rate..." />
)}
{stakingFeeSat ? (
<p>
Transaction fee amount:{" "}
<strong>
{satoshiToBtc(stakingFeeSat)} {coinName}
</strong>
</p>
) : (
<LoadingSmall text="Loading transaction fee amount..." />
)}
<p>
Transaction fee amount:{" "}
<strong>
{satoshiToBtc(stakingFeeSat)} {coinName}
</strong>
</p>
</div>
<button
className="btn btn-sm btn-link no-underline"
Expand Down
69 changes: 33 additions & 36 deletions src/app/components/Staking/Staking.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ export const Staking: React.FC<StakingProps> = ({
overTheCapRange: false,
approchingCapRange: false,
});
const [stakingFeeSat, setStakingFeeSat] = useState<number>(0);
const [isSufficientBalance, setIsSufficientBalance] = useState(false);

// Mempool fee rates, comes from the network
// Fetch fee rates, sat/vB
Expand Down Expand Up @@ -242,6 +244,10 @@ export const Staking: React.FC<StakingProps> = ({

// Either use the selected fee rate or the fastest fee rate
const feeRate = selectedFeeRate || defaultFeeRate;
// check that selected Fee rate (if present) is bigger than the min fee
if (feeRate && feeRate < minFeeRate) {
throw new Error("Selected fee rate is lower than the hour fee");
}

const queryClient = useQueryClient();

Expand Down Expand Up @@ -325,24 +331,18 @@ export const Staking: React.FC<StakingProps> = ({
});
};

// Memoize the staking fee calculation
const stakingFeeSat = useMemo(() => {
useEffect(() => {
if (
btcWalletNetwork &&
address &&
publicKeyNoCoord &&
stakingAmountSat &&
finalityProvider &&
paramWithCtx?.currentVersion &&
mempoolFeeRates &&
feeRate &&
availableUTXOs
) {
try {
// check that selected Fee rate (if present) is bigger than the min fee
if (selectedFeeRate && selectedFeeRate < minFeeRate) {
throw new Error("Selected fee rate is lower than the hour fee");
}
const memoizedFeeRate = selectedFeeRate || defaultFeeRate;
// Calculate the staking fee
const { stakingFeeSat } = createStakingTx(
paramWithCtx.currentVersion,
Expand All @@ -352,25 +352,25 @@ export const Staking: React.FC<StakingProps> = ({
btcWalletNetwork,
address,
publicKeyNoCoord,
memoizedFeeRate,
feeRate,
availableUTXOs,
);
return stakingFeeSat;
setIsSufficientBalance(true);
setStakingFeeSat(stakingFeeSat);
} catch (error: Error | any) {
// fees + staking amount can be more than the balance
showError({
error: {
message: error.message,
errorState: ErrorState.STAKING,
errorTime: new Date(),
},
retryAction: () => setSelectedFeeRate(0),
});
setSelectedFeeRate(0);
return 0;
// This indicates the user has insufficient funds to cover staking amount + fees
if (error?.message.includes("Insufficient funds")) {
setIsSufficientBalance(false);
} else {
showError({
error: {
message: error.message,
errorState: ErrorState.STAKING,
errorTime: new Date(),
},
});
}
}
} else {
return 0;
}
}, [
btcWalletNetwork,
Expand All @@ -380,12 +380,9 @@ export const Staking: React.FC<StakingProps> = ({
stakingTimeBlocks,
finalityProvider,
paramWithCtx,
mempoolFeeRates,
selectedFeeRate,
availableUTXOs,
showError,
defaultFeeRate,
minFeeRate,
feeRate,
]);

// Select the finality provider from the list
Expand Down Expand Up @@ -581,6 +578,7 @@ export const Staking: React.FC<StakingProps> = ({
stakingAmountSat,
stakingTimeBlocksWithFixed,
!!finalityProvider,
isSufficientBalance,
);

const previewReady =
Expand All @@ -604,16 +602,15 @@ export const Staking: React.FC<StakingProps> = ({
btcWalletBalanceSat={btcWalletBalanceSat}
onStakingAmountSatChange={handleStakingAmountSatChange}
reset={resetFormInputs}
isSufficientBalance={isSufficientBalance}
/>
<StakingFee
mempoolFeeRates={mempoolFeeRates}
stakingFeeSat={stakingFeeSat}
selectedFeeRate={selectedFeeRate}
onSelectedFeeRateChange={setSelectedFeeRate}
reset={resetFormInputs}
/>
{signReady && (
<StakingFee
mempoolFeeRates={mempoolFeeRates}
stakingFeeSat={stakingFeeSat}
selectedFeeRate={selectedFeeRate}
onSelectedFeeRateChange={setSelectedFeeRate}
reset={resetFormInputs}
/>
)}
</div>
{showApproachingCapWarning()}
<span
Expand Down
6 changes: 6 additions & 0 deletions src/utils/isStakingSignReady.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const isStakingSignReady = (
amount: number,
time: number,
fpSelected: boolean,
sufficientBalance: boolean,
): { isReady: boolean; reason: string } => {
if (!fpSelected)
return {
Expand Down Expand Up @@ -39,6 +40,11 @@ export const isStakingSignReady = (
isReady: false,
reason: "Please enter a valid staking period",
};
} else if (!sufficientBalance) {
return {
isReady: false,
reason: "Not enough balance to cover the staking value and its fees",
};
}
return {
isReady: true,
Expand Down
14 changes: 12 additions & 2 deletions tests/utils/isStakingSignReady.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { isStakingSignReady } from "@/utils/isStakingSignReady";

describe("utils/isStakingSignReady", () => {
it("should return false with reason if fpSelected is false", () => {
const result = isStakingSignReady(1, 2, 3, 4, 5, 6, false);
const result = isStakingSignReady(1, 2, 3, 4, 5, 6, false, true);
expect(result.isReady).toBe(false);
expect(result.reason).toBe("Please select a finality provider");
});
Expand Down Expand Up @@ -44,6 +44,7 @@ describe("utils/isStakingSignReady", () => {
input.amount,
6,
true,
true,
);
expect(result.isReady).toBe(false);
expect(result.reason).toBe("Please enter a valid stake amount");
Expand Down Expand Up @@ -87,14 +88,23 @@ describe("utils/isStakingSignReady", () => {
5,
input.time,
true,
true,
);
expect(result.isReady).toBe(false);
expect(result.reason).toBe("Please enter a valid staking period");
});
});

it("should return false with reason if sufficientBalance is false", () => {
const result = isStakingSignReady(1, 10, 20, 30, 5, 25, true, false);
expect(result.isReady).toBe(false);
expect(result.reason).toBe(
"Not enough balance to cover the staking value and its fees",
);
});

it("should return true with empty reason if amount and time are ready", () => {
const result = isStakingSignReady(1, 10, 20, 30, 5, 25, true);
const result = isStakingSignReady(1, 10, 20, 30, 5, 25, true, true);
expect(result.isReady).toBe(true);
expect(result.reason).toBe("");
});
Expand Down