backend issue => estimate fee backend only if account has not balance #211
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@MSghais is attempting to deploy a commit to the keep-starknet-strange Team on Vercel. A member of the Team first needs to authorize it. |
|
It would be better to check that the connected account can pay the transaction fees instead of having balance. So IMO it should stay as it is |
We can also add the estimate claim fees on the UI too for the check if it's gonna be done by the UI or backend. |
| /** @TODO add more check for the two claim flow */ | ||
| // Default min to 0 => Change to a constant or UI estimate base claim | ||
| // Nonce first account tx => deduced it never do a tx before. |
There was a problem hiding this comment.
Unnecessary comments can be removed
| /** @TODO add more check for the two claim flow */ | ||
| // Default min to 0 => Change to a constant or UI estimate base claim | ||
| // Nonce first account tx => deduced it never do a tx before. | ||
| const minAmountBalanceToClaim = BigInt(0); |
There was a problem hiding this comment.
This can be removed since we can directly use it in the check, no need for extra variable
| // Default min to 0 => Change to a constant or UI estimate base claim | ||
| // Nonce first account tx => deduced it never do a tx before. | ||
| const minAmountBalanceToClaim = BigInt(0); | ||
| let nonce: undefined | string; |
There was a problem hiding this comment.
No need for nonce check. It is useless. Either the account has balance, or not.
There was a problem hiding this comment.
The nonce let us check if user have no funds because it's a new account (0 tx) or no
| @@ -62,7 +65,62 @@ export const Tips: React.FC = () => { | |||
| }); | |||
| const balance = uint256.uint256ToBN({low: balanceLow, high: balanceHigh}); | |||
|
|
|||
| if (balance < fee) { | |||
| // Send the claim through the wallet | |||
| if (!estimateFee?.overall_fee) { | ||
| try { | ||
| nonce = await connectedAccount?.account?.getNonce(); | ||
| } catch (error) { | ||
| console.log('Get nonce error => maybe first tx', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Again, no need for nonce check.
| let estimateFee; | ||
| try { | ||
| estimateFee = await connectedAccount?.account?.estimateInvokeFee({ | ||
| contractAddress: ESCROW_ADDRESSES[await provider.getChainId()], | ||
| entrypoint: Entrypoint.CLAIM, | ||
| calldata: claimCalldata, | ||
| }); | ||
| } catch (error) { | ||
| console.log("Can't estimate UI fees", error); | ||
| } |
There was a problem hiding this comment.
Error needs to be handled. Either remove the check, or show a warning etc.
| const isBackendNeeded = | ||
| (!nonce && !estimateFee) || | ||
| (estimateFee?.overall_fee && balance < estimateFee?.overall_fee) || | ||
| (nonce && Number(nonce) > 0 && balance <= minAmountBalanceToClaim); |
There was a problem hiding this comment.
Check is too complex, only check needed is either the account has balance, or not.
I've one backend issue (CORS) to estimate the fee through the backend, in this case also the UI claim flow is broken.
Changes:
Try estimate fees and get nonce on the UI.
Do estimate claim fees with backend only if no balance ETH in the account.
Check few things for run backend process claim: