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

Use traces to extract the input for settlement::Observer #3233

Merged
merged 8 commits into from
Jan 15, 2025

Conversation

sunce86
Copy link
Contributor

@sunce86 sunce86 commented Jan 10, 2025

Description

Implements first part of #3177

The expectation for "auction_id" remain the same: auction_id is expected at the end of the /settle call/sub_call.

Changes

  • Uses trace_transaction function to get traces and extract the calldata of settle call

How to test

Existing tests show that no regression bugs were introduced.

Final e2e test should cover this part of the code. Although by manually testing the trace_transaction I am pretty sure the code is correct.

@@ -29,8 +29,6 @@ pub struct Settlement {
gas: eth::Gas,
/// The effective gas price of the settlement transaction.
gas_price: eth::EffectiveGasPrice,
/// The address of the solver that submitted the settlement transaction.
solver: eth::Address,
Copy link
Contributor Author

@sunce86 sunce86 Jan 10, 2025

Choose a reason for hiding this comment

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

Not strictly related to the task but removed for completeness, so we don't have to rethink how this field should be populated.

@sunce86 sunce86 marked this pull request as ready for review January 10, 2025 18:33
@sunce86 sunce86 requested a review from a team as a code owner January 10, 2025 18:33
Copy link
Contributor

@MartinquaXD MartinquaXD left a comment

Choose a reason for hiding this comment

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

Logic looks good to me.
The bit where the auction id gets spliced off the settle() call might have to change in the future depending on the helper contracts. If the auction id gets appended to the overall function call the solidity logic of the helper contracts doesn't have to handle it at all which would probably result in gas savings. Not sure if those are big enough to matter in practice, though.

crates/autopilot/src/domain/settlement/transaction/mod.rs Outdated Show resolved Hide resolved
@sunce86
Copy link
Contributor Author

sunce86 commented Jan 15, 2025

Refactored the error handling to notify more loudly if execution client bug is detected.

Also, added the unit test to verify that transfer calls that emit settlement event are still properly handled: https://etherscan.io/tx/0x030623e438f28446329d8f4ff84db897907fcac59b9943b31b7be66f23c877af

@sunce86 sunce86 enabled auto-merge (squash) January 15, 2025 12:20
@sunce86 sunce86 merged commit 2725d5c into main Jan 15, 2025
11 checks passed
@sunce86 sunce86 deleted the get-input-from-traces branch January 15, 2025 12:26
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants