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

Add block number to mempool_executed logs #3200

Merged
merged 5 commits into from
Jan 6, 2025
Merged

Add block number to mempool_executed logs #3200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3e452c3
Add block number to mempool_executed logs
m-lord-renkse Jan 1, 2025
176aecd
rework comment
m-lord-renkse Jan 2, 2025
0a81a79
Extend error messages
m-lord-renkse Jan 2, 2025
c68e67f
Merge branch 'main' into add-block-no-mempool-executed-logs
m-lord-renkse Jan 3, 2025
5846dbb
Merge branch 'main' into add-block-no-mempool-executed-logs
squadgazzz Jan 6, 2025
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
24 changes: 16 additions & 8 deletions crates/driver/src/domain/mempools.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl Mempools {
// settlement. This way we only run iterations in blocks that can potentially
// include the settlement.
let mut block_stream = into_stream(self.ethereum.current_block().clone());
block_stream.next().await;
let block = block_stream.next().await;

// The tx is simulated before submitting the solution to the competition, but a
// delay between that and the actual execution can cause the simulation to be
Expand All @@ -116,7 +116,7 @@ impl Mempools {
?err,
"settlement tx simulation reverted before submitting to the mempool"
);
return Err(Error::SimulationRevert);
return Err(Error::SimulationRevert(block.map(|block| block.number)));
} else {
tracing::warn!(
?err,
Expand All @@ -142,7 +142,12 @@ impl Mempools {
});
match receipt {
TxStatus::Executed => return Ok(hash.clone()),
TxStatus::Reverted => return Err(Error::Revert(hash.clone())),
TxStatus::Reverted => {
return Err(Error::Revert {
tx_id: hash.clone(),
block_number: block.number,
squadgazzz marked this conversation as resolved.
Show resolved Hide resolved
})
}
TxStatus::Pending => {
// Check if the current block reached the submission deadline block number
if block.number >= submission_deadline {
Expand Down Expand Up @@ -172,7 +177,7 @@ impl Mempools {
?err,
"tx started failing in mempool, cancelling"
);
return Err(Error::SimulationRevert);
return Err(Error::SimulationRevert(Some(block.number)));
} else {
tracing::warn!(?hash, ?err, "couldn't re-simulate tx");
}
Expand Down Expand Up @@ -235,10 +240,13 @@ pub enum RevertProtection {

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("Mined reverted transaction: {0:?}")]
Revert(eth::TxId),
#[error("Simulation started reverting during submission")]
SimulationRevert,
#[error("Mined reverted transaction: {tx_id:?}, block number: {block_number}")]
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)

#[error("Simulation started reverting during submission, block number: {0:?}")]
SimulationRevert(Option<BlockNo>),
#[error("Settlement did not get included in time")]
Expired,
#[error("Strategy disabled for this tx")]
Expand Down
4 changes: 2 additions & 2 deletions crates/driver/src/infra/notify/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ pub fn executed(
) {
let kind = match res {
Ok(hash) => notification::Settlement::Success(hash.clone()),
Err(Error::Revert(hash)) => notification::Settlement::Revert(hash.clone()),
Err(Error::SimulationRevert) => notification::Settlement::SimulationRevert,
Err(Error::Revert { tx_id: hash, .. }) => notification::Settlement::Revert(hash.clone()),
Err(Error::SimulationRevert { .. }) => notification::Settlement::SimulationRevert,
Err(Error::Expired) => notification::Settlement::Expired,
Err(Error::Other(_) | Error::Disabled) => notification::Settlement::Fail,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/infra/observe/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ pub fn mempool_executed(
}
let result = match res {
Ok(_) => "Success",
Err(mempools::Error::Revert(_) | mempools::Error::SimulationRevert) => "Revert",
Err(mempools::Error::Revert { .. } | mempools::Error::SimulationRevert { .. }) => "Revert",
Err(mempools::Error::Expired) => "Expired",
Err(mempools::Error::Other(_)) => "Other",
Err(mempools::Error::Disabled) => "Disabled",
Expand Down
Loading