Add flashtestations builder transaction #268

avalonche · 2025-09-18T18:16:44Z

📝 Summary

Replaces --debug-url with --quote_provider arg
Adds the flashtestations block proof at the end of the block

💡 Motivation and Context

✅ I have completed the following steps:

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

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

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

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

SozinM · 2025-09-22T17:37:55Z

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

+    let tee_address_bytes: [u8; 20] = tee_service_signer.address.into();
+
+    // Calculate keccak256 hash of empty bytes (32 bytes)
+    let ext_data = Bytes::from(b"");


it is always empty?

https://github.com/flashbots/rollup-boost/blob/main/specs/flashtestations.md#extended-registration-data empty for now as we don't have operator keys

SozinM · 2025-09-22T17:42:27Z

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

+    // Builder proof version
+    builder_proof_version: u8,
+    // Whether the workload and address has been registered
+    registered: Arc<AtomicBool>,


OnceLock would be nice in here, but it's nit

SozinM · 2025-09-22T17:48:48Z

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

+
+        if !self.registered.load(Ordering::Relaxed) {
+            info!(target: "flashtestations", "tee service not registered yet, attempting to register");
+            self.set_registered(state_provider, ctx);


What is the purpose of this? If we set_registered to true in here we would still create tx in register_tee_service_tx, right?

this function simulates the registration in the evm and we want to minimise the number of calls to this by only checking while we are not registered. The self.register_tee_service_tx will return None if there is an already registered error from the evm

I see that side effect of set_registered is self.registered.store(registered, Ordering::Relaxed);
Should we do let (register_tx, _) = self.register_tee_service_tx(ctx, &mut evm)?; first and then set_registered?
Or maybe we should do self.set_registered(state_provider, ctx); and then check self.registered again, in case it's registered already and we don't need to register

Should we do let (register_tx, _) = self.register_tee_service_tx(ctx, &mut evm)?; first and then set_registered

the order doesn't matter since self.register_tee_service_tx(ctx, &mut evm)? simulates against the block state that has been built thus far and self.set_registered is simulated against the committed state. this is to prevent registered set to true when the register tx is committed at the top of the block and then ends up not being included in the canonical chain. It would also not guard against reorgs

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

SozinM · 2025-09-22T17:56:07Z

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

+                let verify_block_proof_tx = self.signed_block_builder_proof_tx(
+                    block_content_hash,
+                    ctx,
+                    gas_used * 64 / 63,


Little bit confused in here
If we want to add buffer of size gas_used / 64, then we need to do
gas_used * 65 / 64

so if the contract call uses 1000 gas and only 63/64 is forwarded:
needed = 1000 gas
forwarded = 1000 * 63 / 64 = 984.375
to adjust the needed gas limit equal to the forwarded amount:
(1000 * 64 / 63) * (63 / 64) = 1000

SozinM · 2025-09-22T17:56:31Z

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

+                let nonce = get_nonce(evm.db_mut(), self.tee_service_signer.address)?;
+                let register_tx = self.signed_register_tee_service_tx(
+                    self.attestation.clone(),
+                    gas_used * 64 / 63, // Due to EIP-150, 63/64 of available gas is forwarded to external calls so need to add a buffer


same as below

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

noot · 2025-09-25T02:35:50Z

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

+                    warn!(target: "flashtestations", %err, "register tee service tx failed");
+                    return Ok(TxSimulateResult::default());


why is it okay to ignore the error here?

they are not fatal errors should be handled separately (nonce, balance, gas issues)

noot · 2025-09-25T02:40:53Z

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

+                warn!(target: "flashtestations", "transaction did not emit TEEServiceRegistered log, FlashtestationRegistry contract address may be incorrect");
+                Ok((None, false))


not propagating the error here seems like it could get messy, why not propagate the errors and handle them in the caller?

only fatal errors were returned as it was being propagated to the payload builder and stopping blocks being built. It should not fail on reverts / operational errors (e.g. insufficient balance).

noot · 2025-09-25T02:45:14Z

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

+        if !self.registered.get().unwrap_or(&false) {
+            info!(target: "flashtestations", "tee service not registered yet, attempting to register");
+            builder_txs.extend(self.fund_tee_service_tx(ctx, &mut evm)?);
+            let (register_tx, _) = self.register_tee_service_tx(ctx, &mut evm)?;


the fact that this can silently fail and self.set_registered(..) is still called seems bad. also, set_registered() also calls register_tee_service_tx` internally, are these duplicate calls?

see #268 (comment)

self.register_tee_service_tx is only a simulation of the potential state that is going into the built block while self.set_registered is simulated against the committed state. We don't want to be stuck in a bad state where registered = true when self.register_tee_service_tx succeeds and the builder block doesn't end up being included, and it never attempts registration again

I'll move set_registered away from generating the transactions to avoid this confusion

noot · 2025-09-25T02:46:29Z

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

+sol!(
+    #[sol(rpc, abi)]
+    #[derive(Debug)]
+    interface IFlashtestationRegistry {


please add link to specific commit

Co-authored-by: Solar Mithril <mikawamp@gmail.com>

Co-authored-by: noot <36753753+noot@users.noreply.github.com>

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

akundaz · 2025-09-26T20:08:32Z

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

+    pub signed_tx: Recovered<OpTransactionSigned>,
+    // whether the transaction should be a top of block or
+    // bottom of block transaction
+    pub is_top_of_block: bool,


If we want to switch to bottom of every flashblock how difficult will it be to change this? Not sure if we want something like this configurable or not but it might be easier to start that way

in construction of each transaction added to builder_tx just flip the boolean

How do you flip it? Is it a code change? Then it needs to be built and released and it's a lot more difficult to change

oh you mean configuring via cli / env? I'm not sure this is something we want to change very often since changes to the builder tx position breaks external integrations / observability. Also the same builder tx have different positions in the block depending on whether the flashblock contract call is enabled or not etc. and the complexity would easily cause misconfiguration.

avalonche requested review from SozinM and karim-agha as code owners September 18, 2025 18:16

akundaz reviewed Sep 18, 2025

View reviewed changes

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

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

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

avalonche force-pushed the flashtestations-builder-tx-rebased branch from fb9ed66 to 64be228 Compare September 19, 2025 21:53

SozinM reviewed Sep 22, 2025

View reviewed changes

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

SozinM reviewed Sep 22, 2025

View reviewed changes

noot reviewed Sep 25, 2025

View reviewed changes

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

noot reviewed Sep 25, 2025

View reviewed changes

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

noot reviewed Sep 25, 2025

View reviewed changes

avalonche and others added 8 commits September 26, 2025 04:16

Add flashtestations builder transaction

5acd847

comments

f865586

clean up errors

54f01cb

clean up cancellation logic

e8bbe3e

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

cd2f145

Co-authored-by: Solar Mithril <mikawamp@gmail.com>

comments

563f005

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

9a621f6

Co-authored-by: noot <36753753+noot@users.noreply.github.com>

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

bf944d7

Co-authored-by: noot <36753753+noot@users.noreply.github.com>

avalonche force-pushed the flashtestations-builder-tx-rebased branch from 6b72d3e to bf944d7 Compare September 25, 2025 18:17

akundaz reviewed Sep 26, 2025

View reviewed changes

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

akundaz reviewed Sep 26, 2025

View reviewed changes

		warn!(target: "flashtestations", %err, "register tee service tx failed");
		return Ok(TxSimulateResult::default());

		warn!(target: "flashtestations", "transaction did not emit TEEServiceRegistered log, FlashtestationRegistry contract address may be incorrect");
		Ok((None, false))

Add flashtestations builder transaction #268

Are you sure you want to change the base?

Add flashtestations builder transaction #268

Uh oh!

Conversation

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

Uh oh!

📝 Summary

💡 Motivation and Context

✅ I have completed the following steps:

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!

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!

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!

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

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!

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!

Uh oh!

avalonche commented Sep 18, 2025 •

edited

Loading

avalonche Sep 25, 2025 •

edited

Loading