Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add consts to Params for individual networks #2396

Merged
merged 1 commit into from
Mar 9, 2024
Merged

Add consts to Params for individual networks #2396

merged 1 commit into from
Mar 9, 2024

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jan 25, 2024 •
edited

Add consts to the Params type for the individual networks.

@github-actions github-actions bot added the C-bitcoin PRs modifying the bitcoin crate label Jan 25, 2024
@coveralls
Copy link

coveralls commented Jan 25, 2024 •
edited

Pull Request Test Coverage Report for Build 8087603014

Details

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 84.136%
Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/consensus/params.rs 0 4 0.0%
Totals Coverage Status
Change from base Build 8086856593: 0.2%
Covered Lines: 19438
Relevant Lines: 23103
💛 - Coveralls

@tcharding
Copy link
Member Author

If this fly's we should do the same for blockdata::constants::genesis_block.

Kixunil
Kixunil reviewed Jan 25, 2024
View reviewed changes
bitcoin/src/consensus/params.rs Outdated Show resolved Hide resolved
bitcoin/src/consensus/params.rs Outdated Show resolved Hide resolved
@tcharding tcharding changed the title Add constructors on Params for individual networks Add consts to Params for individual networks Jan 27, 2024
@tcharding
Copy link
Member Author

I'm not sure if this PR adds any value if we are not deprecating Params::new()?

@apoelstra
Copy link
Member

The constants would be nice to have, but yeah, the value is small.

@tcharding
Copy link
Member Author

I'll leave this open then, looks right to go.

@tcharding
Add consts to Params for individual networks
3a56ecc 
Add consts to the `Params` type for the individual networks.
@tcharding
Copy link
Member Author

Re-based on master, I'm not exactly sure what the state of CI is, but this had no acks so seems ok to rebase.

@apoelstra
Copy link
Member

I think CI was fixed by #2512 (there was some clippy lint related to Default) and should be "permanently" fixed by #2489.

apoelstra
apoelstra approved these changes Feb 28, 2024
View reviewed changes
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 3a56ecc

Kixunil
Kixunil approved these changes Feb 29, 2024
View reviewed changes
Copy link
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 3a56ecc

bitcoin/src/consensus/params.rs Show resolved Hide resolved
sanket1729
sanket1729 approved these changes Mar 8, 2024
View reviewed changes
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 3a56ecc

@apoelstra apoelstra merged commit f69417f into rust-bitcoin:master Mar 9, 2024
187 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kixunil Kixunil Kixunil approved these changes

@apoelstra apoelstra apoelstra approved these changes

@sanket1729 sanket1729 sanket1729 approved these changes

Assignees
No one assigned
Labels
C-bitcoin PRs modifying the bitcoin crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tcharding @coveralls @apoelstra @Kixunil @sanket1729