-
Notifications
You must be signed in to change notification settings - Fork 38
Remove non permit flashtestation calls #302
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes deprecated non-permit flashtestation calls and marks the old funding-based approach for deprecation. The changes streamline the flashtestation system to use only permit-based signatures for TEE service registration and block proofs.
Key changes:
- Removed funding-based TEE registration flow in favor of permit-only approach
- Eliminated funding key parameter and related funding transactions
- Removed cleanup tasks for returning funds from TEE-generated keys
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/op-rbuilder/src/tests/flashtestations.rs | Removed test attributes for deprecated non-permit tests; added flashtestations_use_permit: true to remaining tests; removed funding_key parameter from test configurations |
| crates/op-rbuilder/src/flashtestations/tx_manager.rs | Replaced funding-based registration with permit-based registration; removed fund_address and clean_up methods; renamed funding_signer to builder_signer; added EIP-2612 permit signature generation |
| crates/op-rbuilder/src/flashtestations/service.rs | Removed funding_key parameter and cleanup task; simplified bootstrap to only handle permit-based registration; removed BuilderContext parameter |
| crates/op-rbuilder/src/flashtestations/builder_tx.rs | Removed deprecated funding and non-permit registration methods; simplified transaction generation to only use permit-based approach; removed funding_key, funding_amount, and use_permit fields |
| crates/op-rbuilder/src/flashtestations/args.rs | Removed funding_key and funding_amount CLI arguments; marked flashtestations_use_permit as deprecated with default changed to true |
| crates/op-rbuilder/src/builders/standard/service.rs | Removed ctx parameter from bootstrap_flashtestations call |
| crates/op-rbuilder/src/builders/flashblocks/service.rs | Removed ctx parameter from bootstrap_flashtestations call |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
3a260c8 to
4f0411e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let quote_bytes = Bytes::from(attestation); | ||
| let wallet = | ||
| PrivateKeySigner::from_bytes(&self.tee_service_signer.secret.secret_bytes().into()) | ||
| PrivateKeySigner::from_bytes(&self.builder_signer.secret.secret_bytes().into()) |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wallet is created from builder_signer but should be created from tee_service_signer to sign the permit registration transaction. The TEE service signer is the one that needs to authorize the registration via permit, not the builder signer.
| PrivateKeySigner::from_bytes(&self.builder_signer.secret.secret_bytes().into()) | |
| PrivateKeySigner::from_bytes(&self.tee_service_signer.secret.secret_bytes().into()) |
| /// DEPRECATED FLAG: Permit-based signatures are now the default and only supported method. | ||
| /// This flag is no longer necessary and will be removed in future versions. | ||
| /// Users should rely on the default permit-based signature workflow. |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states this flag is deprecated, but it's still functional and being used. If the flag truly has no effect (since permit is now always used), consider removing it entirely or adding a runtime warning when it's set to false to inform users of the deprecation.
aed9dd4 to
0de3c2d
Compare
akundaz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just check the copilot comment on changing tee_service_signer to builder_signer
|
that's the intended wallet for signing the transaction, tee signer signs the message |
π Summary
Remove non permit calls and mark to be deprecated
π‘ Motivation and Context
β I have completed the following steps:
make lintmake test