Implement boundless adapter #1356
Conversation
auto-assign
bot
requested review from
eyusufatik,
jfldde,
kpp,
rakanalh and
yaziciahmet
| @@ -0,0 +1,51 @@ | |||
| [package] | |||
There was a problem hiding this comment.
Sorry but i am very opinionated about introducing crates. Especially this when we have risc0-bonsai and risc0-boundless where both can live in a single proving-clients crate or something like that.
| use tracing::{debug, error, info, instrument, trace, warn}; | ||
| use url::Url; | ||
|
|
||
| // TODO: Remove blocking wrapper once the boundless client has blocking api |
There was a problem hiding this comment.
This TODO seems to be in the wrong place...
| proof_market_address: Address, | ||
| set_verifier_address: Address, | ||
| ) -> Self { | ||
| macro_rules! unwrap_boundless_response { |
There was a problem hiding this comment.
Move macro outside the the impl block.
| ); | ||
| } | ||
| let (queue, mut rx) = tokio::sync::mpsc::unbounded_channel(); | ||
| let join_handle = tokio::spawn(async move { |
There was a problem hiding this comment.
Move this async block into it's own function...
| let join_handle = tokio::spawn(async move { | |
| let join_handle = tokio::spawn(handle_response(client)) |
| .await | ||
| .unwrap(); | ||
|
|
||
| 'queue: loop { |
There was a problem hiding this comment.
- Is it a good idea to retry indefinitely?
- Wouldn't it be better if somehow we return an error that the call-site can handle in a backoff-retry fashion or maybe keep
pendinglist of requests and retry later? - Handle errors in the call-site for more control over what happens if this keeps failing.
| self.queue | ||
| .send(BoundlessRequest::UploadImg { elf, notify }) | ||
| .unwrap(); | ||
| tokio::task::block_in_place(|| rx.blocking_recv().unwrap()) |
There was a problem hiding this comment.
Why use block_in_place when you can make this method async + you're calling this method from an async block?
| fn upload_input_to_boundless(&mut self, input: &[u8]) { | ||
| let client = self.client.clone(); | ||
|
|
||
| // Retry backoff |
There was a problem hiding this comment.
Is this a TODO? Otherwise, this is not retry-backoff
eyusufatik
added
HOLD-MERGE
Description
Implements risc0 boundless adapter
cannot merge yet since boundless repos require ssh access
Linked Issues