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.

Sign up for GitHub

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

Jump to bottom

backend issue => estimate fee backend only if account has not balance #211

Closed
wants to merge 4 commits into from
Closed

backend issue => estimate fee backend only if account has not balance #211

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d90a899
backend issue => estimate fee backend only if account has not balance
MSghais Jul 5, 2024
42ba26a
ui estimate fee => maybe check also nonce too
MSghais Jul 5, 2024
aa09e68
if estimate fee UI failed => do check 0 balance and nonce 0
MSghais Jul 5, 2024
330cf0d
trycatch and do different check for backend process
MSghais Jul 5, 2024
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
90 changes: 61 additions & 29 deletions JoyboyCommunity/src/screens/Tips/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
const event = new NDKEvent(ndk);
event.kind = NDKKind.Text;
event.content = `claim: ${cairo.felt(depositId)},${cairo.felt(
connectedAccount.address!,

Check warning on line 47 in JoyboyCommunity/src/screens/Tips/index.tsx

View workflow job for this annotation

GitHub Actions / check-app 

Forbidden non-null assertion
)},${cairo.felt(ETH[CHAIN_ID].address)},${gasAmount.toString()}`;
event.tags = [];

Expand All @@ -52,8 +52,11 @@
return event.rawEvent();
};

const feeResult = await estimateClaim.mutateAsync(await getNostrEvent(BigInt(1)));
const fee = BigInt(feeResult.data.fee);
/** @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.
Comment on lines +55 to +57
Copy link
Collaborator

@ugur-eren ugur-eren Jul 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Unnecessary comments can be removed

const minAmountBalanceToClaim = BigInt(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed since we can directly use it in the check, no need for extra variable

let nonce: undefined | string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for nonce check. It is useless. Either the account has balance, or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The nonce let us check if user have no funds because it's a new account (0 tx) or no


const [balanceLow, balanceHigh] = await provider.callContract({
contractAddress: ETH[CHAIN_ID].address,
Expand All @@ -62,7 +65,62 @@
});
const balance = uint256.uint256ToBN({low: balanceLow, high: balanceHigh});

if (balance < fee) {
// Send the claim through the wallet
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment can be removed


const event = await getNostrEvent(BigInt(0));

const signature = event.sig ?? '';
const signatureR = signature.slice(0, signature.length / 2);
const signatureS = signature.slice(signature.length / 2);

const claimCalldata = CallData.compile([
uint256.bnToUint256(`0x${event.pubkey}`),
event.created_at,
event.kind ?? 1,
byteArray.byteArrayFromString(JSON.stringify(event.tags)),
{
deposit_id: cairo.felt(depositId),
starknet_recipient: connectedAccount.address,
gas_token_address: ETH[CHAIN_ID].address,
gas_amount: uint256.bnToUint256(0),
},
{
r: uint256.bnToUint256(`0x${signatureR}`),
s: uint256.bnToUint256(`0x${signatureS}`),
},
uint256.bnToUint256(0),
]);

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);
}
Comment on lines +94 to +103
Copy link
Collaborator

Choose a reason for hiding this comment

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

Error needs to be handled. Either remove the check, or show a warning etc.


if (!estimateFee?.overall_fee) {
try {
nonce = await connectedAccount?.account?.getNonce();
} catch (error) {
console.log('Get nonce error => maybe first tx', error);
}
}
Comment on lines +105 to +111
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, no need for nonce check.


/** @TODO better check for backed call need */
// - if nonce is undefined (no tx send) and no estimate fee on the UI
// - Balance < estimate fee
// - Nonce is ok (First tx send) but no balance min
const isBackendNeeded =
(!nonce && !estimateFee) ||
(estimateFee?.overall_fee && balance < estimateFee?.overall_fee) ||
(nonce && Number(nonce) > 0 && balance <= minAmountBalanceToClaim);
Comment on lines +117 to +120
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check is too complex, only check needed is either the account has balance, or not.

if (isBackendNeeded) {
const feeResult = await estimateClaim.mutateAsync(await getNostrEvent(BigInt(1)));
const fee = BigInt(feeResult.data.fee);
// Send the claim through backend

const claimResult = await claim.mutateAsync(await getNostrEvent(fee));
Expand All @@ -82,32 +140,6 @@
}
});
} else {
// Send the claim through the wallet

const event = await getNostrEvent(BigInt(0));

const signature = event.sig ?? '';
const signatureR = signature.slice(0, signature.length / 2);
const signatureS = signature.slice(signature.length / 2);

const claimCalldata = CallData.compile([
uint256.bnToUint256(`0x${event.pubkey}`),
event.created_at,
event.kind ?? 1,
byteArray.byteArrayFromString(JSON.stringify(event.tags)),
{
deposit_id: cairo.felt(depositId),
starknet_recipient: connectedAccount.address,
gas_token_address: ETH[CHAIN_ID].address,
gas_amount: uint256.bnToUint256(0),
},
{
r: uint256.bnToUint256(`0x${signatureR}`),
s: uint256.bnToUint256(`0x${signatureS}`),
},
uint256.bnToUint256(0),
]);

const receipt = await sendTransaction({
calls: [
{
Expand Down
Loading