-
-
Notifications
You must be signed in to change notification settings - Fork 267
Fix/balance change calculation gas sponsored #7608
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 |
|---|---|---|
|
|
@@ -57,6 +57,7 @@ export type GetBalanceChangesRequest = { | |
| getSimulationConfig: GetSimulationConfig; | ||
| nestedTransactions?: NestedTransactionMetadata[]; | ||
| txParams: TransactionParams; | ||
| isGasFeeSponsored?: boolean; | ||
| }; | ||
|
|
||
| type ParsedEvent = { | ||
|
|
@@ -206,11 +207,10 @@ function getNativeBalanceChange( | |
| return undefined; | ||
| } | ||
|
|
||
| return getSimulationBalanceChange( | ||
| previousBalance, | ||
| newBalance, | ||
| transactionResponse.gasCost, | ||
| ); | ||
| // For sponsored transactions, withGas: false ensures stateDiff excludes gas costs. | ||
| // For non-sponsored transactions, withGas: true includes gas costs in stateDiff. | ||
| // Hence gas cost not needed here. | ||
| return getSimulationBalanceChange(previousBalance, newBalance); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -637,15 +637,15 @@ function extractLogs( | |
| * | ||
| * @param previousBalance - The previous balance. | ||
| * @param newBalance - The new balance. | ||
| * @param offset - Optional offset to apply to the new balance. | ||
| * @param offset - Optional offset to apply to the new balance (as BN to maintain precision). | ||
| * @returns The balance change data or undefined if unchanged. | ||
| */ | ||
| function getSimulationBalanceChange( | ||
| previousBalance: Hex, | ||
| newBalance: Hex, | ||
| offset: number = 0, | ||
| offset: BN = new BN(0), | ||
| ): SimulationBalanceChange | undefined { | ||
| const newBalanceBN = hexToBN(newBalance).add(new BN(offset)); | ||
| const newBalanceBN = hexToBN(newBalance).add(offset); | ||
|
Member
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. The purpose of this offset is to remove any gas costs from the native balance change. Do we know why this calculation isn't working if sponsored? Is that the root of the issue rather than the Is the |
||
| const previousBalanceBN = hexToBN(previousBalance); | ||
| const differenceBN = newBalanceBN.sub(previousBalanceBN); | ||
| const isDecrease = differenceBN.isNeg(); | ||
|
|
@@ -742,7 +742,7 @@ async function baseRequest({ | |
| ...params, | ||
| getSimulationConfig, | ||
| transactions, | ||
| withGas: true, | ||
| withGas: !request.isGasFeeSponsored, | ||
|
Member
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. We specify gas here to ensure a more accurate simulation that can't be as easily detected. Why exactly would we not want that if sponsored if we mock the native balance below if insufficient? |
||
| withDefaultBlockOverrides: true, | ||
| ...(blockTime && { | ||
| blockOverrides: { | ||
|
|
||
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.
Do we not want to keep the gas cost in for non-sponsored here?