Add permit flashtestations tx calls from builder #285

avalonche · 2025-10-03T21:34:07Z

📝 Summary

Add permit function calls so that tee signer don't need funding to do the flashtestation transactions. Adds additional flashtestation tests and improves error handling.

CLI Changes

--flashtestations.use-permit enables the permit contract call and does not attempt to fund the tee key. It would use the builder key and many flags such as --flashtestations.rpc-url, --flashtestations.funding-key will not be needed to support flashtestations

✅ I have completed the following steps:

Run make lint
Run make test
Added tests (if applicable)

crates/op-rbuilder/src/builders/builder_tx.rs

Copilot

Pull Request Overview

Adds permit-based registration and block proof verification for flashtestations so the TEE signer no longer needs funding; introduces new tests and refactors builder transaction simulation to support permit flows.

Introduces EIP-712 style permit functions on registry and builder policy contracts plus related signature/nonce logic.
Refactors builder transaction trait and implementations to generic over extra context and execution info, adding unified simulation helpers and error types.
Adds CLI flag --flashtestations.use-permit and corresponding test coverage for permit registration and block proof flows.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
crates/op-rbuilder/src/tests/mod.rs	Updates test contract addresses used in setup.
crates/op-rbuilder/src/tests/flashtestations.rs	Expands test suite with permit, invalid quote, unauthorized workload, and block proof scenarios; adjusts expected tx counts.
crates/op-rbuilder/src/flashtestations/service.rs	Extends bootstrap to pass builder key and support permit flag gating RPC registration.
crates/op-rbuilder/src/flashtestations/mod.rs	Adds permit-related function and error definitions to interfaces; simplifies revert enum.
crates/op-rbuilder/src/flashtestations/builder_tx.rs	Implements permit registration and block proof signing logic; refactors simulation paths and error handling.
crates/op-rbuilder/src/flashtestations/args.rs	Adds CLI arg flashtestations_use_permit.
crates/op-rbuilder/src/builders/standard/service.rs	Passes builder key into flashtestations bootstrap with new signature.
crates/op-rbuilder/src/builders/standard/builder_tx.rs	Adjusts trait usage for new generics and error-tolerant flashtestations integration.
crates/op-rbuilder/src/builders/mod.rs	Re-exports new error/result types, removes obsolete helper.
crates/op-rbuilder/src/builders/flashblocks/service.rs	Updates generic bounds to include execution info type for flashblocks.
crates/op-rbuilder/src/builders/flashblocks/payload.rs	Adds FlashblocksExecutionInfo; updates trait bounds accordingly.
crates/op-rbuilder/src/builders/flashblocks/builder_tx.rs	Adapts builder tx structures to new generic trait; commits state before flashtestations txs.
crates/op-rbuilder/src/builders/builder_tx.rs	Major refactor: adds unified simulation utilities, new error variants, generic BuilderTxBase, and removes log_exists helper.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/op-rbuilder/src/builders/builder_tx.rs

crates/op-rbuilder/src/flashtestations/builder_tx.rs

crates/op-rbuilder/src/tests/flashtestations.rs

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/op-rbuilder/src/tests/flashtestations.rs

crates/op-rbuilder/src/flashtestations/builder_tx.rs

crates/op-rbuilder/src/tests/flashtestations.rs

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/op-rbuilder/src/flashtestations/builder_tx.rs

SozinM · 2025-10-22T15:22:38Z

crates/op-rbuilder/src/builders/builder_tx.rs


+#[derive(Debug, thiserror::Error)]
+pub enum InvalidContractDataError {
+    #[error("did not find expected logs expected {0:?} but got {1:?}")]


Won't :? for vec look unreadable?

Its a vec of hashes though, not a complex struct

SozinM · 2025-10-22T15:26:38Z

crates/op-rbuilder/src/builders/builder_tx.rs

    ) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError>;

-    fn add_builder_txs<Extra: Debug + Default>(
+    fn simulate_builder_txs_with_state_copy(


Doc comment please, why state copy is needed?

SozinM · 2025-10-22T15:36:32Z

crates/op-rbuilder/src/builders/builder_tx.rs


 pub fn get_balance(
-    db: &mut State<impl Database>,
+    db: impl DatabaseRef,


SozinM · 2025-10-22T15:38:07Z

crates/op-rbuilder/src/builders/builder_tx.rs


 #[derive(Debug, Clone)]
-pub(super) struct BuilderTxBase {
+pub(super) struct BuilderTxBase<ExtraCtx = ()> {


How do you use extraCtx in here?

needed in ctx to satisfy generics for both standard and flashblocks builder. Though we can consider deprecating the standard builder now that flashblocks is stable?

SozinM · 2025-10-22T15:43:15Z

crates/op-rbuilder/src/builders/builder_tx.rs

            let ResultAndState { state, .. } = evm
-                .transact(&builder_tx.signed_tx)
+                .transact(&signed_tx)
                .map_err(|err| BuilderTransactionError::EvmExecutionError(Box::new(err)))?;


Do we want to fail all transaction is some tx failed?

BuilderTransactionError::EvmExecutionError is a fatal error, though we can distinguish between invalid transactions (e.g. nonce errors) from evm execution errors and only skip invalid transactions

SozinM · 2025-10-22T15:44:21Z

crates/op-rbuilder/src/builders/flashblocks/builder_tx.rs


        if ctx.is_first_flashblock() {
-            let flashblocks_builder_tx = self.base_builder_tx.simulate_builder_tx(ctx, db)?;
+            let flashblocks_builder_tx = self.base_builder_tx.simulate_builder_tx(ctx, &mut *db)?;


can you just pass db?

SozinM · 2025-10-22T15:46:24Z

crates/op-rbuilder/src/builders/flashblocks/builder_tx.rs

+                        self.flashblock_number_address,
+                        InvalidContractDataError::InvalidLogs(
+                            vec![IFlashblockNumber::FlashblockIncremented::SIGNATURE_HASH],
+                            vec![],


Do we want to put actual logs in there?

no planning to deprecate this soon after permit changes are tested and merged

SozinM · 2025-10-22T15:50:45Z

crates/op-rbuilder/src/flashtestations/args.rs

+        env = "FLASHTESTATIONS_USE_PERMIT",
+        default_value = "false"
+    )]
+    pub flashtestations_use_permit: bool,


Interesting, is there a case when we don't want to use permit?
Maybe it should be the only option for flashtestations?

The non permit flags are being used in experimental so it'll be a breaking change. This PR should be merged first before updating the builder args in experimental, then removing the flags and related code

SozinM · 2025-10-22T15:52:11Z

crates/op-rbuilder/src/flashtestations/builder_tx.rs

-    }
-
-    fn simulate_verify_block_proof_tx<ExtraCtx: Debug + Default>(
+    // TODO: deprecate in favour of permit calls


Maybe it makes sense to deprecate it right now?

Its still being used in experimental

SozinM · 2025-10-22T15:52:46Z

crates/op-rbuilder/src/flashtestations/builder_tx.rs

    ) -> Result<Option<BuilderTransactionCtx>, BuilderTransactionError> {
-        let balance = get_balance(evm.db_mut(), self.tee_service_signer.address)?;
+        let balance = get_balance(evm.db(), self.tee_service_signer.address)?;
        if balance.is_zero() {


Hm, what if balance 1 wei?

we just manually fund it, we don't want to fund the key more than necessary

SozinM · 2025-10-22T15:52:57Z

crates/op-rbuilder/src/flashtestations/builder_tx.rs

    }

-    fn register_tee_service_tx<ExtraCtx: Debug + Default>(
+    // TODO: deprecate in favour of permit calls


Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-22T19:00:38Z

crates/op-rbuilder/src/flashtestations/builder_tx.rs

+    fn fund_tee_service_tx(
        &self,
-        block_content_hash: B256,
        ctx: &OpPayloadBuilderCtx<ExtraCtx>,
-        evm: &mut OpEvm<
-            &mut State<StateProviderDatabase<impl StateProvider>>,
-            NoOpInspector,
-            PrecompilesMap,
-        >,
-    ) -> Result<TxSimulateResult, BuilderTransactionError> {
-        let nonce = get_nonce(evm.db_mut(), self.tee_service_signer.address)?;
-
-        let verify_block_proof_tx = self.signed_block_builder_proof_tx(
-            block_content_hash,
-            ctx,
-            ctx.block_gas_limit(),
-            nonce,
-        )?;
-        let ResultAndState { result, state } = match evm.transact(&verify_block_proof_tx) {
-            Ok(res) => res,
-            Err(err) => {
-                if err.is_invalid_tx_err() {
-                    return Err(BuilderTransactionError::InvalidTransactionError(Box::new(
-                        err,
-                    )));
-                } else {
-                    return Err(BuilderTransactionError::EvmExecutionError(Box::new(err)));
-                }
-            }
-        };
-        match result {
-            ExecutionResult::Success { gas_used, logs, .. } => Ok(TxSimulateResult {
-                gas_used,
-                success: true,
-                state_changes: state,
-                revert_reason: None,
-                logs,
-            }),
-            ExecutionResult::Revert { output, gas_used } => {
-                let revert_reason =
-                    IBlockBuilderPolicy::IBlockBuilderPolicyErrors::abi_decode(&output)
-                        .map(FlashtestationRevertReason::BlockBuilderPolicy)
-                        .unwrap_or_else(|e| {
-                            FlashtestationRevertReason::Unknown(hex::encode(output), e)
-                        });
-                Ok(TxSimulateResult {
-                    gas_used,
-                    success: false,
-                    state_changes: state,
-                    revert_reason: Some(revert_reason),
-                    logs: vec![],
-                })
-            }
-            ExecutionResult::Halt { reason, .. } => Ok(TxSimulateResult {
-                gas_used: 0,
-                success: false,
-                state_changes: state,
-                revert_reason: Some(FlashtestationRevertReason::Halt(reason)),
-                logs: vec![],
-            }),
-        }
-    }
-
-    fn check_verify_block_proof_log(&self, logs: &[Log]) -> bool {
-        for log in logs {
-            if log.topics().first() == Some(&BlockBuilderProofVerified::SIGNATURE_HASH) {
-                return true;
-            }
-        }
-        false
-    }
-
-    fn fund_tee_service_tx<ExtraCtx: Debug + Default>(
-        &self,
-        ctx: &OpPayloadBuilderCtx<ExtraCtx>,
-        evm: &mut OpEvm<
-            &mut State<StateProviderDatabase<impl StateProvider>>,
-            NoOpInspector,
-            PrecompilesMap,
-        >,
+        evm: &mut OpEvm<&mut State<impl Database + DatabaseRef>, NoOpInspector, PrecompilesMap>,
    ) -> Result<Option<BuilderTransactionCtx>, BuilderTransactionError> {


The TODO comment on line 140 indicates this function should be deprecated, but it's still being actively used in the codebase. Consider adding a deprecation timeline or tracking issue to the TODO comment to make it actionable.

Copilot · 2025-10-22T19:00:38Z

crates/op-rbuilder/src/flashtestations/builder_tx.rs

+    fn register_tee_service_tx(
        &self,
        ctx: &OpPayloadBuilderCtx<ExtraCtx>,
-        evm: &mut OpEvm<
-            &mut State<StateProviderDatabase<impl StateProvider>>,
-            NoOpInspector,
-            PrecompilesMap,
-        >,
-    ) -> Result<(Option<BuilderTransactionCtx>, bool), BuilderTransactionError> {
-        let TxSimulateResult {
+        evm: &mut OpEvm<&mut State<impl Database + DatabaseRef>, NoOpInspector, PrecompilesMap>,
+    ) -> Result<BuilderTransactionCtx, BuilderTransactionError> {


The TODO comment on line 186 indicates this function should be deprecated, but it's still actively used in non-permit code paths. Consider adding a deprecation timeline or migration plan to the TODO comment.

Copilot · 2025-10-22T19:00:39Z

crates/op-rbuilder/src/flashtestations/builder_tx.rs

+    fn verify_block_proof_tx(
        &self,
        transactions: Vec<OpTransactionSigned>,
        ctx: &OpPayloadBuilderCtx<ExtraCtx>,
-        evm: &mut OpEvm<
-            &mut State<StateProviderDatabase<impl StateProvider>>,
-            NoOpInspector,
-            PrecompilesMap,
-        >,
-    ) -> Result<Option<BuilderTransactionCtx>, BuilderTransactionError> {
+        evm: &mut OpEvm<impl Database + DatabaseRef, NoOpInspector, PrecompilesMap>,
+    ) -> Result<BuilderTransactionCtx, BuilderTransactionError> {


The TODO comment on line 227 says 'remove in favour of permit calls' but this function is still used in the non-permit code path. Consider clarifying whether this is a planned future removal or if both code paths will be maintained long-term.

Copilot · 2025-10-22T19:00:39Z

crates/op-rbuilder/src/flashtestations/service.rs

+    // TODO: support permit with an external rpc, skip this step if using permit signatures
+    // since the permit txs are signed by the builder key and will result in nonce issues


This TODO describes a nonce issue limitation but doesn't provide a tracking issue or implementation plan. Consider adding a link to a GitHub issue or more specific implementation details about how external RPC support could be added.

Suggested change

// TODO: support permit with an external rpc, skip this step if using permit signatures

// since the permit txs are signed by the builder key and will result in nonce issues

// TODO: Support permit with an external RPC. See tracking issue: https://github.com/your-org/your-repo/issues/123

// Implementation plan: If using permit signatures, skip onchain registration since permit txs are signed by the builder key and will result in nonce issues.

// Instead, add logic to detect permit mode and bypass TxManager registration, relying on external RPC for permit validation and nonce management.

Copilot · 2025-10-22T19:00:39Z

crates/op-rbuilder/src/builders/flashblocks/builder_tx.rs

        }
    }

+    // TODO: remove and clean up in favour of simulate_call()


The TODO suggests removing this function in favor of simulate_call(), but the function is still actively used. Consider adding a timeline or tracking issue to make this TODO actionable, or update the implementation to use simulate_call() if feasible.

Suggested change

// TODO: remove and clean up in favour of simulate_call()

// TODO: remove and clean up in favour of simulate_call().

// Tracking issue: https://github.com/your-org/your-repo/issues/1234

avalonche requested review from SozinM and karim-agha as code owners October 3, 2025 21:34

avalonche force-pushed the flashtestations-permit branch 3 times, most recently from ad61301 to 75e520c Compare October 4, 2025 09:19

SozinM reviewed Oct 4, 2025

View reviewed changes

crates/op-rbuilder/src/builders/builder_tx.rs Show resolved Hide resolved

avalonche mentioned this pull request Oct 6, 2025

Add flashtestation builder tx #282

Merged

3 tasks

avalonche force-pushed the add-flashtestation-integration-tests branch from 260cb95 to 2bd3b66 Compare October 16, 2025 18:21

avalonche requested a review from 0x416e746f6e as a code owner October 16, 2025 18:21

Base automatically changed from add-flashtestation-integration-tests to main October 16, 2025 18:34

Copilot AI review requested due to automatic review settings October 18, 2025 06:32

avalonche force-pushed the flashtestations-permit branch from 75e520c to ef81cca Compare October 18, 2025 06:32

Copilot AI reviewed Oct 18, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings October 18, 2025 10:27

avalonche force-pushed the flashtestations-permit branch from 95a824c to 82c5fea Compare October 18, 2025 10:27

Copilot AI reviewed Oct 18, 2025

View reviewed changes

avalonche force-pushed the flashtestations-permit branch from 82c5fea to e79a2c9 Compare October 18, 2025 10:32

akundaz reviewed Oct 20, 2025

View reviewed changes

crates/op-rbuilder/src/tests/flashtestations.rs Outdated Show resolved Hide resolved

crates/op-rbuilder/src/tests/flashtestations.rs Outdated Show resolved Hide resolved

crates/op-rbuilder/src/tests/flashtestations.rs Outdated Show resolved Hide resolved

avalonche added 4 commits October 22, 2025 02:52

Add permit flashtestations tx calls from builder

2587c0b

move simumlation calls to builder tx

d5bddbd

refactor contract simulation

f757014

refactor flashtestations builder tx

9f2874a

Copilot AI review requested due to automatic review settings October 21, 2025 15:52

avalonche force-pushed the flashtestations-permit branch from e79a2c9 to 9f2874a Compare October 21, 2025 15:52

Copilot AI reviewed Oct 21, 2025

View reviewed changes

crates/op-rbuilder/src/flashtestations/builder_tx.rs Show resolved Hide resolved

crates/op-rbuilder/src/flashtestations/builder_tx.rs Show resolved Hide resolved

crates/op-rbuilder/src/flashtestations/builder_tx.rs Show resolved Hide resolved

fix test comments

4a14be2

SozinM reviewed Oct 22, 2025

View reviewed changes

crates/op-rbuilder/src/builders/builder_tx.rs

pub fn get_balance(

db: &mut State<impl Database>,

db: impl DatabaseRef,

Copy link

Collaborator

SozinM Oct 22, 2025

Choose a reason for hiding this comment

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

noice

SozinM reviewed Oct 22, 2025

View reviewed changes

SozinM approved these changes Oct 22, 2025

View reviewed changes

comments

42df8eb

Copilot AI review requested due to automatic review settings October 22, 2025 18:59

avalonche enabled auto-merge (squash) October 22, 2025 19:00

Copilot AI reviewed Oct 22, 2025

View reviewed changes

avalonche merged commit 663b981 into main Oct 22, 2025
4 checks passed

avalonche deleted the flashtestations-permit branch October 22, 2025 19:20

		// TODO: support permit with an external rpc, skip this step if using permit signatures
		// since the permit txs are signed by the builder key and will result in nonce issues

-    // TODO: support permit with an external rpc, skip this step if using permit signatures
-    // since the permit txs are signed by the builder key and will result in nonce issues
+    // TODO: Support permit with an external RPC. See tracking issue: https://github.com/your-org/your-repo/issues/123
+    // Implementation plan: If using permit signatures, skip onchain registration since permit txs are signed by the builder key and will result in nonce issues.
+    // Instead, add logic to detect permit mode and bypass TxManager registration, relying on external RPC for permit validation and nonce management.

	// TODO: remove and clean up in favour of simulate_call()
	// TODO: remove and clean up in favour of simulate_call().
	// Tracking issue: https://github.com/your-org/your-repo/issues/1234

Add permit flashtestations tx calls from builder #285

Add permit flashtestations tx calls from builder #285

Uh oh!

Conversation

avalonche commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Summary

CLI Changes

✅ I have completed the following steps:

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

avalonche commented Oct 3, 2025 •

edited

Loading