[Feature] Implement StateVerifiedClientStatus Lotus-compatible RPC. #3817
Conversation
|
@ruseinov you can verify with https://github.com/filecoin-project/builtin-actors/tree/v12.0.0 |
ruseinov
requested review from
lemmih and
aatifsyed
and removed request for
a team
| let id = self.lookup_id(addr, ts)?.expect("actor not found"); | ||
| let network_version = self.get_network_version(ts.epoch()); | ||
|
|
||
| // This is a copy of Lotus code, we need to treat all the actors below version 9 |
There was a problem hiding this comment.
wibni: link to a tagged file + line number - helps if someone needs to update the port
src/tool/subcommands/api_cmd.rs
Outdated
| // Those proven hard to verify as most of the time both lotus and forest return 'null'. So it's | ||
| // hardcoded. |
There was a problem hiding this comment.
this doesn't quite make sense to me - consider rewording?
There was a problem hiding this comment.
This note implies understanding of snapshot tests, which takes n last tipsets and tries to verify the results for both Lotus and Forest using the addresses found. Since last n tipsets data doesn't return anything but null for both - the addresses are hardcoded.
Co-authored-by: Aatif Syed <[email protected]>
|
Have you tested if this also works with the new |
I have not, should I? Not sure if it's at all relevant as long as the format is the same. If you didn't have to introduce any changes to other endpoints for said flag - I don't expect this one needing any special treatment. Otherwise let me know what needs to be done. |
No, it was just by curiosity. The test of this flag should be done on the CI anyway.
FYI, you can test it in your case with: |
src/state_manager/mod.rs
Outdated
| addr: &Address, | ||
| ts: &Arc<Tipset>, | ||
| ) -> anyhow::Result<Option<DataCap>> { | ||
| let id = self.lookup_id(addr, ts)?.expect("actor not found"); |
There was a problem hiding this comment.
Shouldn't we rather propagate the error? We wouldn't like to crash the node in case in case of a missing actor, no?
There was a problem hiding this comment.
Yeah, you are right. We should, I will just convert the None value to an error.
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
work on #3639
Other information and links
Tested with:
cargo run --bin=forest-tool -- api compare --filter StateVerifiedClientStatus --lotus /dns/127.0.0.1/tcp/1234/http snapshot-latest.I have found a way to verify at least one of the two Lotus codepaths, so that's something.
Change checklist