-
Notifications
You must be signed in to change notification settings - Fork 161
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
feat: implement Filecoin.StateAccountKey RPC API #3735
Conversation
src/shim/state_tree.rs
Outdated
.get_actor(&addr)? | ||
.with_context(|| format!("failed to find actor: {addr}"))?; | ||
|
||
// if state.Version() >= types.StateTreeVersion5 |
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.
// if state.Version() >= types.StateTreeVersion5 |
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.
This is the Go code. Rust api does not expose state tree version so I have to use some workaround here
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.
So let's better have a comment than a commented-out Go code. :)
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.
Fixed
if let Some(address) = actor.delegated_address { | ||
return Ok(address.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.
Do we even need to check if it's FVM4 or FVM3? What if (when) we get FVM5?
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.
That's good point, it's better to check older versions
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.
Fixed
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.
But do we need to have this check at all? I mean, paranoid checks are fine by me, but I believe the delegated_address
should be None
for older versions.
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.
But do we need to have this check at all? I mean, paranoid checks are fine by me, but I believe the
delegated_address
should beNone
for older versions.
I think you are right, but I'm inclined to keep the logic the same with Go, what do you think?
if state.Version() >= types.StateTreeVersion5 {
if act.Address != nil {
// If there _is_ an f4 address, return it as "key" address
return *act.Address, nil
}
}
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.
Sure, it's a cheap operation anyway.
Summary of changes
As part of #3639
Changes introduced in this pull request:
(Manually changed the code to compare the last 500 tipsets)
Lotus code:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist