State API - RPC methods #532
Conversation
StaticallyTypedAnxiety
requested review from
ansermino,
austinabell,
dutterbutter and
ec2
as code owners
|
You mention this PR closes #476 but I don't see it getting hooked up to JSONRPC as done here https://github.com/ChainSafe/forest/blob/main/node/rpc/src/lib.rs#L30 |
blockchain/state_manager/src/lib.rs
Outdated
| } | ||
|
|
||
| while back_search_wait.next().await.is_some() { | ||
| if !reverts.get(back_tipset.key()).unwrap_or(&false) |
There was a problem hiding this comment.
This gets called for all () put through this pubsub channel, but this back_tipset never changes, would this not just loop over and call the same code on the tipset when the back_tipset was initiated?
There was a problem hiding this comment.
I thought since it is in a loop structure the same thing would be happening at the lotus side? If multiple backsearchWait events are being called the same code will potentially be called as well?
There was a problem hiding this comment.
Well, they are just calling that function asyncronously and only handling it once (and not recurring, they kinda use that channel as a oneshot. Seems like if you have this subscriber you could potentially be receiving events from another function call, unrelated to the logic of this.
I don't see why these events would need to be published in a pubsub way, seems pretty local (maybe clarification around why this needs to be emitted like this would be great, as I'm confused)
There was a problem hiding this comment.
Definitely! Wanted to use a oneshot but it looks like subheadchanges is the one that is sending these events externally? That is where the tsub starts to match in the case? I was also confused as to why they need to be externally called but following lotus seems like that might be the way they want it? I could also be very confused it is not as easy to parse haha
There was a problem hiding this comment.
Yes, I see they are using that and that is matched above, but this back search is completely distinct and you are using the same subscriber channel, this will just keep iterating over and never close, so this function will never end. Am I missing something here?
There was a problem hiding this comment.
this is my lack of go showing, I just recently knew that go func() is supposed to be an anoymous routine. Taking a look at this and fixing this right now, was trying to visualize as to what point the back_search_wait gets sent to and this makes more sense now
vm/message/src/lib.rs
Outdated
| //turns message into cid | ||
| fn to_cid(&self) -> Result<Cid, String>; |
There was a problem hiding this comment.
bumping this comment in case it was missed
blockchain/state_manager/src/lib.rs
Outdated
| } | ||
|
|
||
| while back_search_wait.next().await.is_some() { | ||
| if !reverts.get(back_tipset.key()).unwrap_or(&false) |
There was a problem hiding this comment.
Yes, I see they are using that and that is matched above, but this back search is completely distinct and you are using the same subscriber channel, this will just keep iterating over and never close, so this function will never end. Am I missing something here?
Co-authored-by: Austin Abell <austinabell8@gmail.com>
Co-authored-by: Austin Abell <austinabell8@gmail.com>
Co-authored-by: Austin Abell <austinabell8@gmail.com>
Co-authored-by: Austin Abell <austinabell8@gmail.com>
…ainSafe/forest into ashanti/state_api_rpc_methods
blockchain/state_manager/src/lib.rs
Outdated
| async { | ||
| back_search_recieved = | ||
| Some(receiver.await.map_err(|e| { | ||
| Error::Other(format!("Could not receieve from channel {:?}", e)) | ||
| })?); | ||
| Ok::<(), Error>(()) | ||
| } | ||
| .await?; | ||
|
|
||
| while let Some(subscriber) = self | ||
| .subscriber | ||
| .clone() | ||
| .ok_or_else(|| { | ||
| Error::Other("State Manager not subscribed to tipset head changes".to_string()) | ||
| })? | ||
| .next() | ||
| .await | ||
| { |
There was a problem hiding this comment.
This should be polled at the same time as the subscriber (both the receiver channel and pubsub receiver are futures) as right now you are waiting for the function to complete, then awaiting on the subscriber events. These changes don't change anything I don't think (unless I'm reading wrong)
| pub fn get_power<DB>( | ||
| state_manager: &StateManager<DB>, | ||
| tipset: &Tipset, | ||
| address: Option<&Address>, | ||
| ) -> Result<(Option<Claim>, Claim), Error> | ||
| where | ||
| DB: BlockStore, | ||
| { | ||
| get_power_raw(state_manager, tipset.parent_state(), address) | ||
| } |
There was a problem hiding this comment.
I'm confused about why this is different from the one attached to the state manager?
There was a problem hiding this comment.
Yeah this was a mistake, shouldn't even be here.
| Ok(miner_actor_state.info) | ||
| } | ||
|
|
||
| pub fn get_miner_deadlines<DB>( |
There was a problem hiding this comment.
where is this function coming from? I'm having a hard time mapping some of these functions in this file
There was a problem hiding this comment.
| @@ -284,7 +284,7 @@ dependencies = [ | |||
| "ipld_blockstore", | |||
| "ipld_hamt", | |||
| "lazy_static", | |||
| "libp2p", | |||
| "libp2p 0.21.1", | |||
There was a problem hiding this comment.
What was the reason for bumping versions? was there something that broke? (wondering if there is something to fix) I tried without the updates and things seemed fine
There was a problem hiding this comment.
Not so sure, I just did a cargo update and found this
There was a problem hiding this comment.
ah yeah, that's what I meant to ask, just wasn't sure why the cargo update was necessary
| let unsigned_box = unsigned.into_iter().map(|s| Box::new(s) as BoxMessage); | ||
| let signed_box = signed.into_iter().map(|s| Box::new(s) as BoxMessage); | ||
|
|
||
| let mut messages = Vec::new(); |
There was a problem hiding this comment.
bump using capacity to avoid reallocations
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #476
Closes #487
Closes #489
Other information and links