-
Notifications
You must be signed in to change notification settings - Fork 46
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
Introduce ParentchainHandler
on untrusted side
#1064
Conversation
ParentchainHandler
struct on untrusted side
ParentchainHandler
struct on untrusted sideParentchainHandler
struct on untrusted side
ParentchainHandler
struct on untrusted sideParentchainHandler
on untrusted side
println!("IntegriTEE Worker v{}", VERSION); | ||
println!("Integritee Worker v{}", VERSION); |
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.
This has been around for a long time 🤣 finally gets fixed now 🎉
impl Sidechain for EnclaveMock { | ||
fn sync_parentchain<ParentchainBlock: ParentchainBlockTrait>( | ||
&self, | ||
_blocks: &[sp_runtime::generic::SignedBlock<ParentchainBlock>], | ||
_nonce: u32, | ||
) -> EnclaveResult<()> { | ||
Ok(()) | ||
} | ||
|
||
fn execute_trusted_calls(&self) -> EnclaveResult<()> { | ||
todo!() | ||
} | ||
} |
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.
This is a matter of contention: Ideally, your mocks only mock a single trait in order to keep them as specific but also as simple as possible. Whenever you only need the Sidechain
trait mocked, you now have to use a mock (EnclaveMock
) that implements more than you potentially want. And it is used by components that require other traits, which creates an indirect dependency.
If, however, all dependents of this trait always need that combination of traits, then it's necessary again (which I'm guessing is the case here?). If so, then we can leave it as is..
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.
Hm. I see. Let's leave as it is now, in light of that the Mock currently only used once and that the Traits themselves probably need a refactoring (parentchain_sync should not be Sidechain specific)
My follow-up question will be offline 😈
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!
As a preparatory step for introducing multiple parentchains in the same worker:
ParentchainBlockSyncer
to aParentchainHandler
which handles the connection of the parentchain into the enclave. This should make it easier to handle multiple parentchains on the untrusted side, since it allows creating oneParentchainHandler
struct per parentchain.SidechainApiMock
implementations withEnclaveBaseMock
No new functionality introduced, refactoring only.
Closes #1048