-
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
move sidechain_importBlock rpc method to trusted rpc #1605
move sidechain_importBlock rpc method to trusted rpc #1605
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.
Thanks a lot for the upstream PR I have nothing to add LGTM!
let worker_api_direct = DirectWorkerApi::new(enclave_url.clone().into()); | ||
match worker_api_direct.get_untrusted_worker_url() { | ||
Ok(untrusted_worker_url) => { | ||
peer_urls.push(untrusted_worker_url); | ||
}, | ||
Err(e) => { | ||
error!( | ||
"Failed to get untrusted worker url (enclave: {}): {:?}", | ||
enclave_url, e | ||
); | ||
}, | ||
} | ||
peer_urls.push(enclave_url.into()); |
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.
Just commenting for the records, we change the to enclave_url
because the server is now running on the trusted side.
let direct_client = DirectWorkerApi::new(url.clone()); | ||
if let Err(e) = direct_client.import_sidechain_blocks(encoded_blocks_cloned) { |
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.
Nice code quality improvement :)
let fetch_sidechain_blocks_module = BlockFetchServerModuleBuilder::new(sidechain_block_fetcher) | ||
.build() | ||
.map_err(|e| CallError::Failed(e.to_string().into()))?; // `to_string` necessary due to no all errors implementing Send + Sync. |
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.
hmmm, I guess the sidechain block fetcher should also be on the trusted side eventually.
The main goal of this PR is to move
sidechain_importBlock
from untrusted rpc endpoint to trusted. By doing so, there is no need to query for untrusted url anymore during peers update. It also:rpc_methods
trusted json-rpc method