- 
                Notifications
    
You must be signed in to change notification settings  - Fork 45
 
[Sidechain] Peer block fetching o-call implementation #619
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
Changes from all commits
2e8c57d
              4fffffe
              8d30d58
              c295a2b
              2c15003
              fd4fa45
              87de715
              62a4ea4
              45b3cd4
              da2f482
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| [package] | ||
| name = "itp-api-client-extensions" | ||
| name = "itp-node-api-extensions" | ||
| version = "0.8.0" | ||
| authors = ["Integritee AG <[email protected]>"] | ||
| edition = "2018" | ||
| 
     | 
||
| [dependencies] | ||
| # crates.io | ||
| codec = { package = "parity-scale-codec", version = "2.0.0", features = ["derive"] } | ||
| thiserror = "1.0" | ||
| 
     | 
||
| # substrate | ||
| sp-core = { version = "4.1.0-dev", git = "https://github.com/paritytech/substrate.git", branch = "master" } | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /* | ||
| Copyright 2021 Integritee AG and Supercomputing Systems AG | ||
| Copyright (C) 2017-2019 Baidu, Inc. All Rights Reserved. | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
| 
     | 
||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move the   | 
||
| use sp_core::sr25519; | ||
| use substrate_api_client::{rpc::WsRpcClient, Api}; | ||
| 
     | 
||
| /// Trait to create a node API, based on a node URL and signer. | ||
| pub trait CreateNodeApi { | ||
| fn create_api(&self) -> Result<Api<sr25519::Pair, WsRpcClient>>; | ||
| } | ||
| 
     | 
||
| /// Node API factory error. | ||
| #[derive(Debug, thiserror::Error)] | ||
| pub enum NodeApiFactoryError { | ||
| #[error("Failed to create a node API: {0}")] | ||
| FailedToCreateNodeApi(#[from] substrate_api_client::ApiClientError), | ||
| #[error(transparent)] | ||
| Other(#[from] Box<dyn std::error::Error + Sync + Send + 'static>), | ||
| } | ||
| 
     | 
||
| pub type Result<T> = std::result::Result<T, NodeApiFactoryError>; | ||
| 
     | 
||
| /// Node API factory implementation. | ||
| pub struct NodeApiFactory { | ||
| node_url: String, | ||
| signer: sr25519::Pair, | ||
| } | ||
| 
     | 
||
| impl NodeApiFactory { | ||
| pub fn new(url: String, signer: sr25519::Pair) -> Self { | ||
| NodeApiFactory { node_url: url, signer } | ||
| } | ||
| } | ||
| 
     | 
||
| impl CreateNodeApi for NodeApiFactory { | ||
| fn create_api(&self) -> Result<Api<sr25519::Pair, WsRpcClient>> { | ||
| Api::<sr25519::Pair, WsRpcClient>::new(WsRpcClient::new(self.node_url.as_str())) | ||
| .map_err(NodeApiFactoryError::FailedToCreateNodeApi) | ||
| .map(|a| a.set_signer(self.signer.clone())) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -21,7 +21,9 @@ pub extern crate alloc; | |
| 
     | 
||
| use alloc::vec::Vec; | ||
| use codec::{Decode, Encode}; | ||
| use itp_types::{TrustedOperationStatus, WorkerRequest, WorkerResponse}; | ||
| use itp_types::{ | ||
| BlockHash, ShardIdentifier, TrustedOperationStatus, WorkerRequest, WorkerResponse, | ||
| }; | ||
| use sgx_types::*; | ||
| use sp_runtime::OpaqueExtrinsic; | ||
| 
     | 
||
| 
          
            
          
           | 
    @@ -75,10 +77,17 @@ pub trait EnclaveSidechainOCallApi: Clone + Send + Sync { | |
| &self, | ||
| signed_blocks: Vec<SignedSidechainBlock>, | ||
| ) -> SgxResult<()>; | ||
| 
     | 
||
| fn store_sidechain_blocks<SignedSidechainBlock: Encode>( | ||
| &self, | ||
| signed_blocks: Vec<SignedSidechainBlock>, | ||
| ) -> SgxResult<()>; | ||
| 
     | 
||
| fn fetch_sidechain_blocks_from_peer<SignedSidechainBlock: Decode>( | ||
| &self, | ||
| last_known_block_hash: BlockHash, | ||
| shard_identifier: ShardIdentifier, | ||
| ) -> SgxResult<Vec<SignedSidechainBlock>>; | ||
| 
         
      Comment on lines
    
      +86
     to 
      +90
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extend the o-call API on the enclave side  | 
||
| } | ||
| 
     | 
||
| /// Newtype for IPFS CID | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -17,9 +17,11 @@ | |
| */ | ||
| 
     | 
||
| use crate::ocall::{ffi, OcallApi}; | ||
| use codec::Encode; | ||
| use codec::{Decode, Encode}; | ||
| use frame_support::ensure; | ||
| use itp_ocall_api::EnclaveSidechainOCallApi; | ||
| use itp_types::{BlockHash, ShardIdentifier}; | ||
| use log::*; | ||
| use sgx_types::{sgx_status_t, SgxResult}; | ||
| use std::vec::Vec; | ||
| 
     | 
||
| 
          
            
          
           | 
    @@ -65,4 +67,40 @@ impl EnclaveSidechainOCallApi for OcallApi { | |
| 
     | 
||
| Ok(()) | ||
| } | ||
| 
     | 
||
| fn fetch_sidechain_blocks_from_peer<SignedSidechainBlock: Decode>( | ||
| &self, | ||
| last_known_block_hash: BlockHash, | ||
| shard_identifier: ShardIdentifier, | ||
| ) -> SgxResult<Vec<SignedSidechainBlock>> { | ||
| let mut rt: sgx_status_t = sgx_status_t::SGX_ERROR_UNEXPECTED; | ||
| let last_known_block_hash_encoded = last_known_block_hash.encode(); | ||
| let shard_identifier_encoded = shard_identifier.encode(); | ||
| 
     | 
||
| // We have to pre-allocate the vector and hope it's large enough | ||
| let mut signed_blocks_encoded: Vec<u8> = vec![0; 4096 * 16]; | ||
| 
         
      Comment on lines
    
      +80
     to 
      +81
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what worries me a bit: In order to get the sidechain blocks over the o-call interface, we have to pre-allocate the vector into which the block data is filled. I think there's no other elegant solution to this? An alternative would be to have an additional o-call to query the size first, so we can then allocate exactly the right amount. But it would basically double the number of calls into our block storage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ouf, you're right. That's quite ugly. I do see a different solution, but that might require some redesigning: Instead of fetching the blocks directly with this ocall, you could start a fetcher function on the untrusted side with it. This function then retrieves the sidechain blocks and buffers them on the untrusted side (not sure if buffering is needed, there might be a simpler solution). For each retrieved sidechain block (or a chunk of sidechain blocks), it would then do an ecall to import the block (I guess that could be the same ecall as for the gossiped sidechain blocks). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The block production suspension would then also need to be lifted via ecall. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I think that would be more complex to do, because we would go from a pull model to a push model (from the enclave perspective), which is not easy to do in the current PeerBlockSyncer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do agree that this is more complex. But simply adjusting the buffer to an arbitrary amount, I also don't think is sensible - except if we limit our sidechain buffering to a reasonable size which we can import in one go. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue created -> #621  | 
||
| 
     | 
||
| let res = unsafe { | ||
| ffi::ocall_fetch_sidechain_blocks_from_peer( | ||
| &mut rt as *mut sgx_status_t, | ||
| last_known_block_hash_encoded.as_ptr(), | ||
| last_known_block_hash_encoded.len() as u32, | ||
| shard_identifier_encoded.as_ptr(), | ||
| shard_identifier_encoded.len() as u32, | ||
| signed_blocks_encoded.as_mut_ptr(), | ||
| signed_blocks_encoded.len() as u32, | ||
| ) | ||
| }; | ||
| 
     | 
||
| ensure!(rt == sgx_status_t::SGX_SUCCESS, rt); | ||
| ensure!(res == sgx_status_t::SGX_SUCCESS, res); | ||
| 
     | 
||
| let decoded_signed_blocks: Vec<SignedSidechainBlock> = | ||
| Decode::decode(&mut signed_blocks_encoded.as_slice()).map_err(|e| { | ||
| error!("Failed to decode WorkerResponse: {}", e); | ||
| sgx_status_t::SGX_ERROR_UNEXPECTED | ||
| })?; | ||
| 
     | 
||
| Ok(decoded_signed_blocks) | ||
| } | ||
| } | ||
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.
After discussing with @haerdib, I decided to rename the crate and move the node api factory into this crate (so it can be re-used)