chore: upgrade vm & crypto#2320
Conversation
| futures = { workspace = true } | ||
| miden-node-proto = { workspace = true } | ||
| miden-node-utils = { workspace = true } | ||
| miden-processor = { workspace = true } # TODO: removable by re-exporting LoadedMastForest from miden-tx |
There was a problem hiding this comment.
Do we want to create an issue for this?
There was a problem hiding this comment.
sergerad
left a comment
sergerad
left a comment
There was a problem hiding this comment.
LGTM. I'm unfamiliar with the MASM and AccountId changes. Also the RocksDbStorage will be removed soon so its not super important.
igamigo
left a comment
igamigo
left a comment
There was a problem hiding this comment.
LGTM! Left some minor comments. Feel free to disregard the nits if you prefer the current approaches.
| let delta = delta.get(); | ||
| let source = format!( | ||
| "begin\n push.{delta} exec.::miden::protocol::tx::update_expiration_block_delta\nend" | ||
| "@transaction_script\npub proc main\n push.{delta} exec.::miden::protocol::tx::update_expiration_block_delta\nend" |
There was a problem hiding this comment.
nit: I think we could write this in a multi-line string to make it just a bit more readable
There was a problem hiding this comment.
Actually I think this would be replaced soon with the usage of ExpirationTransactionScript so no need to do anything (or maybe change this to use that if it's simple enough?)
| miden-node-proto = { workspace = true } | ||
| miden-node-proto-build = { features = ["internal"], workspace = true } | ||
| miden-node-utils = { workspace = true } | ||
| miden-processor = { workspace = true } # TODO: removable by re-exporting LoadedMastForest from miden-tx |
There was a problem hiding this comment.
I see that this can be addressed now as the PR was merged, leaving this comment just to track that
| .compile_component_code("benchmark::storage_map", "pub proc noop push.0 drop end") | ||
| .compile_component_code( | ||
| "benchmark::storage_map", | ||
| "@account_procedure pub proc noop push.0 drop end", |
There was a problem hiding this comment.
nit: This reads a bit weird now so maybe it could be laid out in multiple lines
There was a problem hiding this comment.
Didn't review this as I assume it's the same as the crypto backend which is getting removed soon. Some name constants appear to not be identical but this should be no problem AFAICT.
I think we are still nuking the database once 0.16 is released, but if we are not there should be a migration path for the persisted trees. cc @Mirko-von-Leipzig @sergerad just in acse
There was a problem hiding this comment.
We are actually removing RocksDbStorage from miden-node, and keeping it in crypto. Opposite of what the linked issue states. I have added a comment to the issue to make sure all are aligned.
3 participants
Summary
Continuation of #2282
Upgrades
miden-vmandmiden-cryptoto its latest versions.Changelog