8000 Move weight constants in the `Weight` type by RCasatta · Pull Request #1826 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Move weight constants in the Weight type #1826

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.

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

Merged
merged 1 commit into from
May 1, 2023

Conversation

RCasatta
Copy link
Collaborator
@RCasatta RCasatta commented May 1, 2023

deprecate constants::MAX_BLOCK_WEIGHT and constants::MIN_TRANSACTION_WEIGHT to nicely redirect users to the constants in the Weight type

@RCasatta RCasatta force-pushed the weight_constants branch 2 times, most recently from 54aff43 to 23ca71f Compare May 1, 2023 09:43
@@ -276,7 +276,7 @@ impl PartialMerkleTree {
return Err(NoTransactions);
};
// check for excessively high numbers of transactions
if self.num_transactions > MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT {
if self.num_transactions as u64 > (Weight::MAX_BLOCK / Weight::MIN_TRANSACTION.to_wu()).to_wu() {
Copy link
Collaborator Author
@RCasatta RCasatta May 1, 2023

Choose a reason for hiding this comment

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

impl Div<Weight> for Weight could have been useful here...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why it's not Weight::MAX_BLOCK.to_wu() / Weight::MIN_TRANSACTION.to_wu()?

Anyway, I wouldn't mind having Div since it properly returns unitless number but it might look a bit strange.

Copy link
< 8000 span data-view-component="true" class="Label ml-1">Collaborator Author

Choose a reason for hiding this comment

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

Why it's not Weight::MAX_BLOCK.to_wu() / Weight::MIN_TRANSACTION.to_wu()?

Yeah that could have been better, I went for the Div impl in the latest version

@RCasatta RCasatta force-pushed the weight_constants branch from 23ca71f to 074ae7b Compare May 1, 2023 11:44
deprecate constants::MAX_BLOCK_WEIGHT and constants::MIN_TRANSACTION_WEIGHT
to nicely redirect users to the constants in the Weight type
@RCasatta RCasatta force-pushed the weight_constants branch from 074ae7b to fc7c251 Compare May 1, 2023 11:55
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 fc7c251

@Kixunil
Copy link
Collaborator
Kixunil commented May 1, 2023

Note that if we're doing Div here we should have it for other units too.

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 fc7c251

@apoelstra apoelstra merged commit 5160a0a into rust-bitcoin:master May 1, 2023
@stevenroose
Copy link
Collaborator

ACK :)

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.

4 participants
0