-
Notifications
You must be signed in to change notification settings - Fork 47
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
Provision light-client in mutual remote attestation #1399
Conversation
…ion import type.
…-production import type." This reverts commit c3c46fd.
…ms such that it can be shared across threads in an arc. [light-client] add constructor to `LightClientStateSealSync` fix light-client adjustments remove pending wip of sealing adjustements
4ae26f2
to
4a2d31e
Compare
for LightClientStateSeal<B, LightClientState> | ||
{ | ||
type LightClientState = LightClientState; |
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.
Associated type is more flexible, as otherwise I would have to declare an extra generic param for the block when being generic of LightClientSealing
because the generic type of the state is generic over the block, which in turn means that I would have had to introduce a phantom type for the block, not funny.
pub unjustified_headers: Vec<Block::Hash>, // Finalized headers without grandpa proof | ||
pub verify_tx_inclusion: Vec<OpaqueExtrinsic>, // Transactions sent by the relay | ||
pub scheduled_change: Option<ScheduledChangeAtBlock<Block::Header>>, // Scheduled Authorities change as indicated in the header's digest. | ||
} | ||
|
||
impl<Block: BlockT> RelayState<Block> { |
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.
Todo: don't define this on the RelayState
itself, rather introduce a new type HeaderQueue
, which implements those methods, and can be unit tested.
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.
@@ -63,12 +61,12 @@ impl FullParachainHandler { | |||
|
|||
let genesis_header = params.genesis_header.clone(); | |||
|
|||
let light_client_seal = EnclaveLightClientSeal::new(base_path.join(LIGHT_CLIENT_DB_PATH))?; | |||
let light_client_seal = GLOBAL_LIGHT_CLIENT_SEAL.get()?; |
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.
It is now initialized at an earlier stage and made available as a global component.
ShieldingKey = 0, | ||
StateKey = 1, | ||
State = 2, | ||
ShieldingKey, | ||
StateKey, | ||
State, | ||
LightClient, |
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.
Clippy gave me warnings here, and as From<u8>
is implemented manually too, I don't think it adds a lot of value. The TLS part should be improved anyhow.
@@ -57,6 +60,7 @@ impl From<u8> for Opcode { | |||
0 => Opcode::ShieldingKey, | |||
1 => Opcode::StateKey, | |||
2 => Opcode::State, | |||
3 => Opcode::LightClient, | |||
_ => unimplemented!("Unsupported/unknown Opcode for MU-RA exchange"), |
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.
We only use this code on the tls-client side during initialization, still this is a runtime panic that could occur. Not nice: #1400
pub struct SealHandler<ShieldingKeyRepository, StateKeyRepository, StateHandler> | ||
where | ||
ShieldingKeyRepository: AccessKey<KeyType = Rsa3072KeyPair> + MutateKey<Rsa3072KeyPair>, | ||
StateKeyRepository: AccessKey<KeyType = Aes> + MutateKey<Aes>, | ||
// Constraint StateT = StfState currently necessary because SgxExternalities Encode/Decode does not work. | ||
// See https://github.com/integritee-network/sgx-runtime/issues/46. | ||
StateHandler: HandleState<StateT = StfState>, | ||
{ |
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.
Anti-pattern: add traitbounds to type definitions when it is not necessary.
Ooops, sorry it seems that I fixed the sidechain demo, but I bricked the indirect invocation. I did not expect that. I have to look into it. |
#[derive(Debug)] | ||
pub struct LightClientStateSealSync<B, LightClientState> { | ||
seal: LightClientStateSeal<B, LightClientState>, | ||
_rw_lock: RwLock<()>, | ||
} | ||
|
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.
#[derive(Debug)] | |
pub struct LightClientStateSealSync<B, LightClientState> { | |
seal: LightClientStateSeal<B, LightClientState>, | |
_rw_lock: RwLock<()>, | |
} | |
// safe to clone into threads. | |
#[derive(Clone, Debug)] | |
pub struct LightClientStateSealSync<B, LightClientState> { | |
seal: Arc<RwLock<LightClientStateSeal<B, LightClientState>>, | |
} | |
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.
Think a bit more before we tackle that: #1401
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 the small things that have to be ironed out are in their own separate issue.
Closes #1370. As the the light-client db is provisioned now, we don't need to wait until the worker has synced the entire parentchain (except for the very first worker). This may greatly speed up the startup of new workers.
Other notes