Skip to content

Conversation

@stevenroose
Copy link
Contributor

The Rust enum stuff is all fancy and such, but it's often a real pain in the ass to either just check which of the vairants is used or to get a field out if you already know what the variant is.

@stevenroose stevenroose force-pushed the params-insepctors branch 2 times, most recently from abd1fe5 to 4155708 Compare September 25, 2019 11:19
@stevenroose stevenroose changed the title Add some inspector methods to dynafed::Params Add some inspector methods to dynafed::Params + support root calculation Sep 25, 2019
@stevenroose stevenroose force-pushed the params-insepctors branch 2 times, most recently from 8462bd3 to 9b047fe Compare September 25, 2019 13:31
};
assert_eq!(
compact_entry.calculate_root().to_hex(),
"dff5f3793abc06a6d75e80fe3cfd47406f732fa4ec9305960ae2a229222a1ad5"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't appear to match elements test hash 4587cf1c2448740c3fe4bf0d3ab22f3f193b18f43c4ee47da3690c53becb7fa4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah I forgot to update those after I changed the test. In this PR you can see the correct ones: https://github.com/ElementsProject/elements/pull/719/files
I'll fix this in a force-push.

Copy link
Collaborator

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

ACK d7d5c5d


#[test]
fn test_param_roots() {
// Taken from the following Elements Core test:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would have been better I think to just include a github link to commit and line number. But not all that important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the test in Elements was still up for review and might not be merged or be changed in the future. I figured it's not that big so better just make sure to have it.

jonasnick added a commit that referenced this pull request Sep 27, 2019
…ot calculation

d7d5c5d Support for calculating consensus params roots (Steven Roose)
2c9c2fb Add some inspector methods to dynafed::Params (Steven Roose)

Pull request description:

  The Rust enum stuff is all fancy and such, but it's often a real pain in the ass to either just check which of the vairants is used or to get a field out if you already know what the variant is.

ACKs for top commit:
  jonasnick:
    ACK d7d5c5d

Tree-SHA512: 50dda4991cce964d6cfe102b20ee12d855565961df41989adb619f8f246bffc40fff8d93a3fed0c23e090d80091b3c94f23d7a0e6c9a59960029b467cf42db1e
@jonasnick jonasnick merged commit d7d5c5d into ElementsProject:master Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants