Skip to content
This repository has been archived by the owner on Nov 9, 2023. It is now read-only.

Update dusk-plonk and canonical and related dependencies #130

Merged
merged 4 commits into from
May 3, 2021

Conversation

CPerezz
Copy link
Contributor

@CPerezz CPerezz commented Apr 30, 2021

  • Update dusk-pki from v0.6 to v0.7.0-rc.0.
  • Update dusk-poseidon from v0.20.0 to v0.21.0-rc.0.
  • Update dusk-bls12_381 from v0.6.0 to v0.8.0-rc.0.
  • Update dusk-jubjub from v0.8.0 to v0.10.0-rc.0.
  • Update dusk-plonk from v0.5 to v0.8.0-rc.1.
  • Update rand-core from v0.5 to v0.6.
  • Update canonical from v0.5 to v0.6.
  • Update rand from v0.7 to v0.8.
  • Update plonk_gadgets from v0.5 to v0.6.0-rc.0.
  • Remove MemStore references from tests.
  • Remove canonical-host from dev-deps.
  • Update tests according to the new dusk-plonk API.
  • Change Bid::pos method to return &u64 instead of u64 directly.

Resolves: #127, #129

- Update `dusk-pki` from `v0.6` to `v0.7.0-rc.0`.
- Update `dusk-poseidon` from `v0.20.0` to `v0.21.0-rc.0`.
- Update `dusk-bls12_381` from `v0.6.0` to `v0.8.0-rc.0`.
- Update `dusk-jubjub` from `v0.8.0` to `v0.10.0-rc.0`.
- Update `dusk-plonk` from `v0.5` to `v0.8.0-rc.1`.
- Update `rand-core` from `v0.5` to `v0.6`.
- Update `canonical` from `v0.5` to `v0.6`.
- Update `rand` from `v0.7` to `v0.8`.
- Update `plonk_gadgets` from `v0.5` to `v0.6.0-rc.0`.
- Remove `MemStore` references from tests.
- Remove `canonical-host` from dev-deps.
- Update tests according to the new `dusk-plonk` API.
- Change `Bid::pos` method to return &u64 instead of `u64` directly.

Resolves: #127
Resolves: #129
@CPerezz CPerezz requested review from ZER0 and vlopes11 and removed request for ZER0 April 30, 2021 11:46
@CPerezz CPerezz added the team:Core Low Level Core Development Team (Rust) label Apr 30, 2021
@CPerezz CPerezz added this to the Port To Canonical 0.6 milestone Apr 30, 2021
Cargo.toml Outdated
anyhow = {version = "1", optional = true}
dusk-bytes = "0.1"
cfg-if = "1.0"

[dev-dependencies]
canonical_host = "0.5"
microkelvin = "0.7.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always use the form major.minor never use the fix:

Suggested change
microkelvin = "0.7.1"
microkelvin = "0.7"

@@ -154,12 +145,11 @@ mod protocol_tests {
let latest_consensus_step = 50u64;

// Append the Bid to the tree.
tree.push(bid.into());
tree.push(bid.into()).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use expect at least, or since the test returns a Result checks if you can use ? or map_err.

.poseidon_branch(0usize)
.expect("Poseidon Branch Extraction");
let branch =
tree.branch(0).expect("Poseidon Branch Extraction").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@@ -237,12 +231,11 @@ mod protocol_tests {
let latest_consensus_step = 25519u64;

// Append the Bid to the tree.
tree.push(bid.into());
tree.push(bid.into()).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

To be clear, if the tests was already using unwrap() I wouldn't care much, but this is a kind of "regression".

.poseidon_branch(0usize)
.expect("Poseidon Branch Extraction");
let branch =
tree.branch(0).expect("Poseidon Branch Extraction").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@@ -331,12 +328,11 @@ mod protocol_tests {
.expect("Bid creation error");

// Append the Bid to the tree.
tree.push(bid.into());
tree.push(bid.into()).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

.poseidon_branch(0usize)
.expect("Poseidon Branch Extraction");
let branch =
tree.branch(0).expect("Poseidon Branch Extraction").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@@ -433,12 +433,11 @@ mod protocol_tests {
.expect("Bid creation error");

// Append the Bid to the tree.
tree.push(bid.into());
tree.push(bid.into()).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

.poseidon_branch(0usize)
.expect("Poseidon Branch Extraction");
let branch =
tree.branch(0).expect("Poseidon Branch Extraction").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

fn poseidon_hash(&self) -> BlsScalar {
self.0.hash()
}

fn pos(&self) -> u64 {
self.0.pos()
*self.0.pos()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already discuss that, and I've mixed feeling about it.

@CPerezz
Copy link
Contributor Author

CPerezz commented May 3, 2021

@ZER0 all of the unwrap mess is done like this because #132 solves it with the correct error handling that I did not want to introduce here so that the PR was not bigger and the scope broader.

@CPerezz CPerezz requested a review from ZER0 May 3, 2021 10:37
@CPerezz CPerezz merged commit 30ff3e9 into release-0.8 May 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
team:Core Low Level Core Development Team (Rust)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants