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

Add block number to mempool_executed logs #3200

Merged
merged 5 commits into from
Jan 6, 2025

Conversation

m-lord-renkse
Copy link
Contributor

Description

Add block number to mempool_executed logs, so we can simulate failed submissions in order to debug the reason of the failure.

@m-lord-renkse m-lord-renkse marked this pull request as ready for review January 1, 2025 11:09
@m-lord-renkse m-lord-renkse requested a review from a team as a code owner January 1, 2025 11:09
crates/driver/src/domain/mempools.rs Outdated Show resolved Hide resolved
crates/driver/src/domain/mempools.rs Outdated Show resolved Hide resolved
crates/driver/src/infra/observe/mod.rs Outdated Show resolved Hide resolved
@squadgazzz
Copy link
Contributor

so we can simulate failed submissions in order to debug the reason of the failure

Which data is expected to be used in order to simulate it(apart from the block number)? An example of the reverted txs: https://aws-es.cow.fi/_dashboards/goto/6d4fcd16a939814d07fbec7d5e72660e?security_tenant=global
It contains 2 txs inside with a huge amount of access lists.

@m-lord-renkse
Copy link
Contributor Author

so we can simulate failed submissions in order to debug the reason of the failure

Which data is expected to be used in order to simulate it(apart from the block number)? An example of the reverted txs: https://aws-es.cow.fi/_dashboards/goto/6d4fcd16a939814d07fbec7d5e72660e?security_tenant=global It contains 2 txs inside with a huge amount of access lists.

The access list is not really needed for the simulation, it saves gas, but it can be simulated without it.

crates/driver/src/domain/mempools.rs Outdated Show resolved Hide resolved
Revert {
tx_id: eth::TxId,
block_number: BlockNo,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can be also (as data types are clear):

#[error("Mined reverted transaction: {0:?}, block: {1:?}")]
Revert(eth::TxId, eth::BlockNo),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sunce86 we show this as a debug version of it. I would keep it as in the logs will say tx_id and block_number, IMO better than seeing only a hashed and a number without saying what it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it's gonna be BlockNo(123456789)

crates/driver/src/domain/mempools.rs Show resolved Hide resolved
@MartinquaXD MartinquaXD merged commit 9b6743a into main Jan 6, 2025
11 checks passed
@MartinquaXD MartinquaXD deleted the add-block-no-mempool-executed-logs branch January 6, 2025 11:37
@github-actions github-actions bot locked and limited conversation to collaborators Jan 6, 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.

5 participants