fvm debug execution #288
fvm debug execution #288
Conversation
|
@vyzo Can you please remove the duplication so we can merge this PR? |
|
yes of course! |
21bf73a to
4effec4
Compare
|
Changes:
Should be ready now. |
There was a problem hiding this comment.
LGTM besides the comments. Would prefer @Stebalien to give the final signoff as a more active maintainer of this repo.
rust/src/fvm/machine.rs
Outdated
| let manifest_cid = match manifest_cid { | ||
| Some(manifest_cid) => { | ||
| let cid = Cid::try_from(&manifest_cid[..]) | ||
| .map_err(|err| anyhow!("invalid manifest: {}", err))?; | ||
| Some(cid) | ||
| } | ||
| // handle cid.Undef for no manifest | ||
| // this can mean two things: | ||
| // - for pre nv16, use the builtin bundles | ||
| // - for nv16 or higher, it means we have already migrated state for system | ||
| // actor and we can pass None to the machine constructor to fish it from state. | ||
| // The presence of the manifest cid argument allows us to test with new bundles | ||
| // with minimum friction. | ||
| None | ||
| None => None, |
There was a problem hiding this comment.
This is a longer version and a bit pedantic with None => None. Since we're changing this, wouldn't it be more succinct to have a manifest_cid.map()?
There was a problem hiding this comment.
map didnt like me, but i can remove the comment completely.
There was a problem hiding this comment.
I've pushed a "rusty" way of doing this, which you can feel free to revert if it triggers something. it's more of a demonstration than anything.
There was a problem hiding this comment.
it came from outer space... this language is an obscurity competition.
There was a problem hiding this comment.
Says the lisper (although APL still wins this round).
| @@ -102,9 +101,28 @@ fn create_fvm_machine( | |||
| Ok(Some(manifest)) => { | |||
| network_config.override_actors(manifest); | |||
| } | |||
| Ok(None) => {} | |||
| Ok(None) => (), | |||
There was a problem hiding this comment.
its equivalent, so its a matter of style; fine with reverting it.
Feel free to revert if this is too rusty.
rust/src/fvm/machine.rs
Outdated
| let manifest_cid = match manifest_cid { | ||
| Some(manifest_cid) => { | ||
| let cid = Cid::try_from(&manifest_cid[..]) | ||
| .map_err(|err| anyhow!("invalid manifest: {}", err))?; | ||
| Some(cid) | ||
| } | ||
| // handle cid.Undef for no manifest | ||
| // this can mean two things: | ||
| // - for pre nv16, use the builtin bundles | ||
| // - for nv16 or higher, it means we have already migrated state for system | ||
| // actor and we can pass None to the machine constructor to fish it from state. | ||
| // The presence of the manifest cid argument allows us to test with new bundles | ||
| // with minimum friction. | ||
| None | ||
| None => None, |
There was a problem hiding this comment.
I've pushed a "rusty" way of doing this, which you can feel free to revert if it triggers something. it's more of a demonstration than anything.
|
I think we are aiming for lotus master -- @arajasek ? |
Companion to filecoin-project/ref-fvm#605