Feat: TEE prover service implementation#345
Conversation
a6bccf7 to
87ca68d
Compare
damiannolan
left a comment
There was a problem hiding this comment.
Nice work, in terms of the prover service integration looks clean enough to me, but I need to get a better understanding of the flow overall.
crates/ev-prover/src/config/mod.rs
Outdated
| Ok(Self { | ||
| grpc_address: "127.0.0.1:50051".into(), | ||
| rpc: RpcConfig::from_env()?, | ||
| hyperlane: HyperlaneConfig::default(), |
There was a problem hiding this comment.
If the intension is to move everything to a load from env or default pattern then shouldn't we use from_env() here also?
There was a problem hiding this comment.
Same comment here, should this be using from_env()?
There was a from_env impl added for CelestiaHyperlaneConfig but I don't see where its being used.
hyperlane/relayer-config.json
Outdated
| "mailbox": "0xb1c938F5BA4B3593377F399e12175e8db0C787Ff", | ||
| "merkleTreeHook": "0x1D957dA7A6988f5a9d2D2454637B4B7fea0Aeea5", | ||
| "protocolFee": "0xFCb1d485ef46344029D9E8A7925925e146B3430E", | ||
| "protocolFee": "0x1D957dA7A6988f5a9d2D2454637B4B7fea0Aeea5", |
There was a problem hiding this comment.
is this change correct? I know it shouldn't technically affect the relayer but both addresses for merkle tree hook and protocol fee hook are the same here.
Relayer doesn't care about protocl fee hook (afaik)
There was a problem hiding this comment.
Same here, this should not change
| @@ -1,5 +1,5 @@ | |||
| defaultHook: | |||
| address: "0xFCb1d485ef46344029D9E8A7925925e146B3430E" | |||
There was a problem hiding this comment.
Same here, I'm not sure that this should change ?
There was a problem hiding this comment.
This comment still is applicable. This address is being changed for some reason to the same address of the requiredHook below, which is a merkle tree hok.
There was a problem hiding this comment.
Can we put this back before merge
damiannolan
left a comment
There was a problem hiding this comment.
Prover impl looks good! Nice work.
There's still some comments that need to be addressed before we merge tho.
- Configuration (see comments on
from_env) - Reading ELFs from expected relative directory
- Hyperlane contract address changes
- Remove any dangling references to "middleware"
crates/ev-prover/src/config/mod.rs
Outdated
| Ok(Self { | ||
| grpc_address: "127.0.0.1:50051".into(), | ||
| rpc: RpcConfig::from_env()?, | ||
| hyperlane: HyperlaneConfig::default(), |
There was a problem hiding this comment.
Same comment here, should this be using from_env()?
There was a from_env impl added for CelestiaHyperlaneConfig but I don't see where its being used.
| use tracing::{debug, error, info, warn}; | ||
|
|
||
| #[derive(Deserialize, Serialize)] | ||
| struct AttestationRequest { |
There was a problem hiding this comment.
nit: can we move this struct after the use crate definition below
There was a problem hiding this comment.
How come we are using hex strings over the wire? Is that the best approach?
| @@ -1,5 +1,5 @@ | |||
| defaultHook: | |||
| address: "0xFCb1d485ef46344029D9E8A7925925e146B3430E" | |||
There was a problem hiding this comment.
This comment still is applicable. This address is being changed for some reason to the same address of the requiredHook below, which is a merkle tree hok.
hyperlane/relayer-config.json
Outdated
| "mailbox": "0xb1c938F5BA4B3593377F399e12175e8db0C787Ff", | ||
| "merkleTreeHook": "0x1D957dA7A6988f5a9d2D2454637B4B7fea0Aeea5", | ||
| "protocolFee": "0xFCb1d485ef46344029D9E8A7925925e146B3430E", | ||
| "protocolFee": "0x1D957dA7A6988f5a9d2D2454637B4B7fea0Aeea5", |
There was a problem hiding this comment.
Same here, this should not change
damiannolan
left a comment
There was a problem hiding this comment.
Approving! Feel free to merge after the contract addresses are put back for protocol fee hook!
| @@ -1,5 +1,5 @@ | |||
| defaultHook: | |||
| address: "0xFCb1d485ef46344029D9E8A7925925e146B3430E" | |||
There was a problem hiding this comment.
Can we put this back before merge
hyperlane/relayer-config.json
Outdated
| "mailbox": "0xb1c938F5BA4B3593377F399e12175e8db0C787Ff", | ||
| "merkleTreeHook": "0x1D957dA7A6988f5a9d2D2454637B4B7fea0Aeea5", | ||
| "protocolFee": "0xFCb1d485ef46344029D9E8A7925925e146B3430E", | ||
| "protocolFee": "0x1D957dA7A6988f5a9d2D2454637B4B7fea0Aeea5", |
This PR adds a prover service implementation for the TEE service.
A new feature flag
tee_modewas introduced alongsidebatch_modethat will start the TEE prover service + message prover service, similar to how we did it for the batch prover.Detailed documentation can be found in
TEE.md.