Migrate to miden-vm v0.11
#528
Conversation
| @@ -15,7 +15,7 @@ resolver = "2" | |||
|
|
|||
| [workspace.package] | |||
| edition = "2021" | |||
| rust-version = "1.80" | |||
| rust-version = "1.82" | |||
There was a problem hiding this comment.
What's causing this bump?
There was a problem hiding this comment.
miden-crypto now require rust 1.82
| @@ -13,7 +13,7 @@ BUILD_PROTO=BUILD_PROTO=1 | |||
|
|
|||
| .PHONY: clippy | |||
| clippy: ## Runs Clippy with configs | |||
| cargo clippy --locked --workspace --all-targets --all-features -- -D warnings | |||
| cargo clippy --locked --workspace --all-targets --all-features -- -D warnings --allow clippy::arc_with_non_send_sync | |||
There was a problem hiding this comment.
This seems really unsafe 😅 But I don't understand these unsafe impl context's so I'll withhold judgement until its been explained.
I would suggest manually #[allow(arc_with_non_send_sync)] on the actual instances instead of blanket allowing this.
There was a problem hiding this comment.
No, it's safe, it's just a suppression of annoying clippy warnings introduced in the latest Rust versions. :) It warns you, if you use Arc where it is sufficient to use just Rc. I think, we should update our code and enable this lint, but in separate PR.
There was a problem hiding this comment.
Let's create an issue to remove this once 0xPolygonMiden/miden-base#909 is fixed.
There was a problem hiding this comment.
Can't comment on this; masm is not in my wheel house.
| @@ -13,7 +13,7 @@ BUILD_PROTO=BUILD_PROTO=1 | |||
|
|
|||
| .PHONY: clippy | |||
| clippy: ## Runs Clippy with configs | |||
| cargo clippy --locked --workspace --all-targets --all-features -- -D warnings | |||
| cargo clippy --locked --workspace --all-targets --all-features -- -D warnings --allow clippy::arc_with_non_send_sync | |||
There was a problem hiding this comment.
Let's create an issue to remove this once 0xPolygonMiden/miden-base#909 is fixed.
There was a problem hiding this comment.
Actually, I pushed a commit which should address #528 (comment), #528 (comment), and #528 (comment).
@Mirko-von-Leipzig, @polydez - take a look and see if this is good to merge.
bin/faucet/src/client.rs
Outdated
| self.faucet_account | ||
| .borrow_mut() | ||
| .lock() | ||
| .unwrap() |
There was a problem hiding this comment.
I think we should avoid using unwrap in favor of expect everywhere except tests if panic is highly unlikely to happen (otherwise, an error should be returned). This helps us to understand why method might fail or why we don't expect it to fail. I usually add .expect("Poisoned lock") for every lock or read/write in case of using RwLock.
bin/faucet/src/client.rs
Outdated
| @@ -60,20 +63,20 @@ impl FaucetClient { | |||
| initialize_faucet_client(config.clone()).await?; | |||
|
|
|||
| let (faucet_account, account_seed, secret) = build_account(config.clone())?; | |||
| let faucet_account = Rc::new(RefCell::new(faucet_account)); | |||
| let id = faucet_account.borrow().id(); | |||
| let faucet_account = faucet_account; | |||
There was a problem hiding this comment.
This line seems to be useless.
|
@Fumuran - could you fix up the remaining comments in this PR? After this, let's merge 0xPolygonMiden/miden-base#929, and then update dependencies and merge this PR as well. |
|
0xPolygonMiden/miden-base#929 has now been merged. Let's update the |
This PR updates the code of the
miden-nodeto work with a next v0.11 version of themiden-vmand next version ofmiden-base.