Skip to content

refactor: encapsulate prep and bump logic #107

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

Merged
merged 9 commits into from
Jun 16, 2025
Merged

Conversation

prestwich
Copy link
Member

@prestwich prestwich commented Jun 13, 2025

Encapsulate logic for the following processes:

  • Tx preparation
  • fee bumping

So that they are out of the main retrying loop

Closes ENG-1084

@prestwich prestwich marked this pull request as ready for review June 13, 2025 15:17
@dylanlott dylanlott force-pushed the prestwich/some-cleaning-submit branch from 56cb4d0 to 628ae83 Compare June 13, 2025 23:55
@dylanlott dylanlott force-pushed the prestwich/bumpable-prep branch from 491b9cd to 0d88be1 Compare June 13, 2025 23:55
@dylanlott dylanlott force-pushed the prestwich/some-cleaning-submit branch from 628ae83 to b5082b8 Compare June 14, 2025 00:10
@dylanlott dylanlott force-pushed the prestwich/bumpable-prep branch 3 times, most recently from 4f0445e to ecc8355 Compare June 14, 2025 02:35

let blob_basefee = prev_header
.next_block_blob_fee(BlobParams::prague())
.expect("signet deployed after 4844 active");
Copy link
Contributor

Choose a reason for hiding this comment

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

So, this expect was panicking at runtime. I updated it to an unwrap_or and it defaults to the Prague value, but I'm not sure why it was panicking.

Copy link
Contributor

@dylanlott dylanlott left a comment

Choose a reason for hiding this comment

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

Tested and working in dev-net with the commits that patch the expect issue. Approving to unblock.

@Evalir Evalir changed the base branch from prestwich/some-cleaning-submit to graphite-base/107 June 16, 2025 14:35
@Evalir Evalir force-pushed the graphite-base/107 branch from 3265307 to da4dd57 Compare June 16, 2025 14:35
@Evalir Evalir force-pushed the prestwich/bumpable-prep branch from f6fb01d to 7e925ff Compare June 16, 2025 14:35
@Evalir Evalir changed the base branch from graphite-base/107 to prestwich/some-cleaning-submit_split June 16, 2025 14:35
@Evalir Evalir changed the base branch from prestwich/some-cleaning-submit_split to graphite-base/107 June 16, 2025 14:41
@Evalir Evalir force-pushed the prestwich/bumpable-prep branch from d02b77e to 244f70d Compare June 16, 2025 14:41
@Evalir Evalir force-pushed the graphite-base/107 branch from da4dd57 to e1f2221 Compare June 16, 2025 14:41
@graphite-app graphite-app bot changed the base branch from graphite-base/107 to main June 16, 2025 14:42
@Evalir Evalir force-pushed the prestwich/bumpable-prep branch from 244f70d to c3d8590 Compare June 16, 2025 14:42
ControlFlow::Retry => {
// bump the req
req = bumpable.bumped();
if bumpable.bump_count() > retry_limit {
Copy link
Member

Choose a reason for hiding this comment

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

this comparison means that we send a TX at most 1 + RETRY_LIMIT times. Usually retries include the first time a tx is sent, but i'm fine with it not being included.

@Evalir Evalir merged commit dd1fd3f into main Jun 16, 2025
6 checks passed
@Evalir Evalir deleted the prestwich/bumpable-prep branch June 16, 2025 15:36
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