Skip to content

fix: submit channel refactors #94

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 11 commits into from
Jun 4, 2025
Merged

Conversation

dylanlott
Copy link
Contributor

@dylanlott dylanlott commented May 22, 2025

fix: submit channel refactors

This PR includes several fixes and refactors to the channel submission logic.

  • Handle 403 errors with Skips instead of erroring
  • Adds retry_count increments to some code paths that weren't previously ticking the count
  • Adds per-slot limiting to the block building loop so that the retry function doesn't simply get called again after 3 attempts
  • Explicitly logs the call results of the blob transaction to aid with debugging
  • Adds gas bumping logic to the submit task to facilitate transaction replacement

Additionally, there are some configuration changes that needed to be made.

  • Additional broadcast hosts was just pointing back at the host RPC, which was adding noise to the logs and interfering with nonce debugging. This PR makes it optional and un-sets the duplicate host-rpc URL.
  • Slot offset and start timestamp were incorrectly configured and have been updated to their correct values

Closes ENG-1079

See: Builder Retry Logic

Copy link
Contributor Author

dylanlott commented May 22, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@dylanlott dylanlott changed the title fix: remove arc and clones fix: submit channel refactors May 22, 2025
@dylanlott dylanlott changed the base branch from main to prestwich/env-cache-integration May 23, 2025 02:57
@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch from 9a88965 to 8a1e80b Compare May 23, 2025 04:05
@dylanlott dylanlott changed the base branch from prestwich/env-cache-integration to main May 23, 2025 04:06
@dylanlott dylanlott changed the base branch from main to prestwich/env-cache-integration May 23, 2025 04:06
@dylanlott dylanlott changed the base branch from prestwich/env-cache-integration to main May 23, 2025 04:07
@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch from 8a1e80b to 0498aae Compare May 23, 2025 17:02
@dylanlott dylanlott changed the base branch from main to prestwich/env-cache-integration May 23, 2025 17:03
@prestwich prestwich force-pushed the prestwich/env-cache-integration branch from a239bcb to 2fd44a7 Compare May 23, 2025 18:02
@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch 2 times, most recently from 444d0f3 to f762131 Compare May 23, 2025 21:38
@dylanlott dylanlott changed the base branch from prestwich/env-cache-integration to main May 23, 2025 21:38
@prestwich prestwich force-pushed the dylan/submit-channel-refactors branch from f762131 to 8c0eaa5 Compare May 27, 2025 12:34
@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch 2 times, most recently from 2d47dc8 to 455791f Compare May 27, 2025 21:48
@dylanlott dylanlott self-assigned this May 27, 2025
@dylanlott dylanlott added the bug Something isn't working label May 27, 2025
@dylanlott dylanlott marked this pull request as ready for review May 27, 2025 22:00
@dylanlott dylanlott requested a review from Evalir as a code owner May 27, 2025 22:00
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

nice progress. some nits:

"error in transaction submission"
);

// NB: These errors are all handled the same way but are logged for debugging purposes
if e.as_revert_data()
Copy link
Member

Choose a reason for hiding this comment

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

style: DRY

macro_rules! check_selector {
    ($e:ident, $rd:ident, $selector:ident, $msg:literal) => {
        if $rd.starts_with(&$selector) { 
            debug!("incorrect host block");
            bail!(e)
        }
    }
}

if let Some(rd) = e.as_revert_data() {
    check_selector!(e, rd, IncorrectHostBlock::SELECTOR, "incorrect host block")
    check_selector!(e, rd, Zenith::BadSignature::SELECTOR, "...")
    ...
    bail!(e)
}

Copy link
Member

Choose a reason for hiding this comment

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

@prestwich prestwich mentioned this pull request May 29, 2025
@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch 3 times, most recently from 9ef1835 to 70591dd Compare May 29, 2025 22:31
@dylanlott dylanlott changed the base branch from main to evalir/chore-deps-upgrade-to-latest-alloy-sdk-trevm May 29, 2025 22:43
Base automatically changed from evalir/chore-deps-upgrade-to-latest-alloy-sdk-trevm to main May 30, 2025 04:22
Cargo.toml Outdated
Comment on lines 28 to 30
init4-bin-base = { version = "0.4.1", features = ["perms"] }

signet-constants = { git = "https://github.com/init4tech/signet-sdk", rev = "b8251ff0fec7cb14ca87e6f95c14f56bc2593049" }
signet-sim = { git = "https://github.com/init4tech/signet-sdk", rev = "b8251ff0fec7cb14ca87e6f95c14f56bc2593049" }
signet-tx-cache = { git = "https://github.com/init4tech/signet-sdk", rev = "b8251ff0fec7cb14ca87e6f95c14f56bc2593049" }
signet-types = { git = "https://github.com/init4tech/signet-sdk", rev = "b8251ff0fec7cb14ca87e6f95c14f56bc2593049" }
signet-zenith = { git = "https://github.com/init4tech/signet-sdk", rev = "b8251ff0fec7cb14ca87e6f95c14f56bc2593049" }
signet-constants = { git = "https://github.com/init4tech/signet-sdk", rev = "ba5894f6fac35299d495ae115cd465a1f0791637" }
Copy link
Member

Choose a reason for hiding this comment

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

note that technically there's a version mismatch here: bin-base will pull signet-constants from commit bd183b627dcb0eb682da801093b13f1f8311446b while this dep will be exclusively using this commit. This shouldn't cause problems right now, but it is something to be aware of. We could bump signet-constants as a patch release to make sure it's at the commit? @prestwich

@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch from 7371bc5 to 0a3a830 Compare May 30, 2025 20:01
@dylanlott dylanlott force-pushed the dylan/submit-channel-refactors branch from 0a3a830 to 0e8eda5 Compare May 30, 2025 21:12
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

just a few nits, but i think we can proceed with this

Comment on lines 28 to +34
init4-bin-base = { version = "0.4.1", features = ["perms"] }

signet-constants = { git = "https://github.com/init4tech/signet-sdk", rev = "bd183b627dcb0eb682da801093b13f1f8311446b" }
signet-sim = { git = "https://github.com/init4tech/signet-sdk", rev = "bd183b627dcb0eb682da801093b13f1f8311446b" }
signet-tx-cache = { git = "https://github.com/init4tech/signet-sdk", rev = "bd183b627dcb0eb682da801093b13f1f8311446b" }
signet-types = { git = "https://github.com/init4tech/signet-sdk", rev = "bd183b627dcb0eb682da801093b13f1f8311446b" }
signet-zenith = { git = "https://github.com/init4tech/signet-sdk", rev = "bd183b627dcb0eb682da801093b13f1f8311446b" }
signet-constants = { git = "https://github.com/init4tech/signet-sdk", rev = "ba5894f6fac35299d495ae115cd465a1f0791637" }
signet-sim = { git = "https://github.com/init4tech/signet-sdk", rev = "ba5894f6fac35299d495ae115cd465a1f0791637" }
signet-tx-cache = { git = "https://github.com/init4tech/signet-sdk", rev = "ba5894f6fac35299d495ae115cd465a1f0791637" }
signet-types = { git = "https://github.com/init4tech/signet-sdk", rev = "ba5894f6fac35299d495ae115cd465a1f0791637" }
signet-zenith = { git = "https://github.com/init4tech/signet-sdk", rev = "ba5894f6fac35299d495ae115cd465a1f0791637" }
Copy link
Member

Choose a reason for hiding this comment

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

let's upgrade this to the 0.2.0 tag (we can now just use tag instead)

Copy link
Member

Choose a reason for hiding this comment

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

unless we need test-utils or rpc, we can actually skip the tag = and just use published?

Copy link
Member

@prestwich prestwich left a comment

Choose a reason for hiding this comment

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

just nits

let block = block_build.build().await;
debug!(block = ?block, "finished block simulation");
let built_block = block_build.build().await;
debug!(block_number = ?built_block.block_number(), "finished building block");
Copy link
Member

Choose a reason for hiding this comment

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

the ? is unnecessary, as block_number is a u64 which is a valid tracing value

@@ -155,8 +155,7 @@ impl Simulator {

// If no env, skip this run
let Some(block_env) = self.block_env.borrow_and_update().clone() else { return };

debug!(block_env = ?block_env, "building on block");
debug!(block_env = ?block_env, "building on block env");
Copy link
Member

Choose a reason for hiding this comment

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

debug!(?block_env, ...)
no need to do = as we're not renaming or invoking any expressions


// manually retrieve nonce
let nonce =
self.provider().get_transaction_count(self.provider().default_signer_address()).await?;
Copy link
Member

Choose a reason for hiding this comment

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

good target for a utility function

let SendableTx::Envelope(tx) = self.provider().fill(tx).await? else {
bail!("failed to fill transaction")
};
debug!(tx_hash = ?tx.hash(), host_block_number = %resp.req.host_block_number, "sending transaction to network");
Copy link
Member

Choose a reason for hiding this comment

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

no need for the % on the host block number. it's a u64, right?

@dylanlott dylanlott merged commit a87da01 into main Jun 4, 2025
6 checks passed
Copy link
Contributor Author

Merge activity

@dylanlott dylanlott deleted the dylan/submit-channel-refactors branch June 4, 2025 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants