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: overwrite gasLeft topic with the value in the original transaction receipt when re-executing a transaction #830

Merged

Conversation

marcospb19-cw
Copy link
Contributor

No description provided.

@marcospb19-cw marcospb19-cw requested a review from a team as a code owner May 13, 2024 18:59
@marcospb19-cw marcospb19-cw force-pushed the copy-gasLeft-from-original-transaction-receipt-on-import branch from 736388e to 354347a Compare May 13, 2024 19:21
@carneiro-cw
Copy link
Contributor

carneiro-cw commented May 13, 2024

Running the log fixing function only in case the comparison fails might be more performatic since it's not common for logs to contain the ERC20Trace event. But I'm not sure if it's worth the extra time spent implementing this :p.

EDIT: Actually, since this affects the mainnet import as well I think I'm more inclined to believe it is worth it.

@marcospb19-cw marcospb19-cw force-pushed the copy-gasLeft-from-original-transaction-receipt-on-import branch 2 times, most recently from 7e0ca48 to 59af95e Compare May 13, 2024 21:32
@marcospb19-cw marcospb19-cw force-pushed the copy-gasLeft-from-original-transaction-receipt-on-import branch 6 times, most recently from 8e514c7 to b6108e1 Compare May 14, 2024 00:27
@dinhani-cw
Copy link
Contributor

dinhani-cw commented May 15, 2024

Please put the fixes based on the receipt inside the new function apply_receipt.

It is called in more than one place.

pub fn apply_receipt(&mut self, receipt: &ExternalReceipt) -> anyhow::Result<()> {

when re-executing the imported transaction, the `gasLeft` topic in logs
diverges from what is received in the receipt, this topic is emmited by
the `ERC20Trace` event

to allow us to keep importing, we workaround this issue by overwriting
the re-execution logs by the value found in the original receipt
@marcospb19-cw marcospb19-cw force-pushed the copy-gasLeft-from-original-transaction-receipt-on-import branch from b6108e1 to c563890 Compare May 15, 2024 21:13
@marcospb19-cw marcospb19-cw merged commit 9420741 into main May 16, 2024
25 checks passed
@marcospb19-cw marcospb19-cw deleted the copy-gasLeft-from-original-transaction-receipt-on-import branch May 16, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants