8000 add MAX_MONEY public constant to Amount by 6293 · Pull Request #742 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

add MAX_MONEY public constant to Amount #742

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
Merged
merged 1 commit into from
Dec 16, 2021
Merged

add MAX_MONEY public constant to Amount #742

merged 1 commit into from
Dec 16, 2021

Conversation

6293
Copy link
Contributor
@6293 6293 commented Dec 14, 2021

Closes #740

Kixunil
Kixunil reviewed Dec 14, 2021
View reviewed changes
8000
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.

Could you add the same constant for unsigned Amount too, please?

Note for other reviewers: Mul::mul is not const so 21_000_000 * Self::ONE_BTC wouldn't work.

@Kixunil Kixunil added trivial Obvious, easy and quick to review (few lines or doc-only...) waiting for author This can only progress if the author responds to a request. labels Dec 14, 2021
@6293
Copy link
Contributor Author
6293 commented Dec 15, 2021

@Kixunil thanks, updated.

@6293 6293 requested a review from Kixunil December 15, 2021 03:22
@sanket1729
Copy link
Member

We should address why @dpc reacted with thumbs down before we proceed on this.

Kixunil
Kixunil requested changes Dec 15, 2021
View reviewed changes
src/util/amount.rs Outdated
@@ -270,6 +270,8 @@ impl Amount {
pub const ONE_SAT: Amount = Amount(1);
/// Exactly one bitcoin.
pub const ONE_BTC: Amount = Amount(100_000_000);
/// The maximum value allowed as an amount. Useful for sanity checking.
pub const MAX_MONEY: SignedAmount = SignedAmount(21_000_000 * 100_000_000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like you forgot to remove Signed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙏

@Kixunil
Copy link
Collaborator
Kixunil commented Dec 15, 2021

@sanket1729 I think he thought it is an implementation of #620 but it isn't. @dpc could you clarify?

@6293 6293 requested a review from Kixunil December 15, 2021 10:02
@Kixunil Kixunil removed the waiting for author This can only progress if the author responds to a request. label Dec 15, 2021
@dpc
Copy link
Contributor
dpc commented Dec 15, 2021

Correct. Please don't mind me.

Kixunil
Kixunil approved these changes Dec 15, 2021
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 ab12410

@Kixunil Kixunil added the one ack PRs that have one ACK, so one more can progress them label Dec 15, 2021
RCasatta
RCasatta approved these changes Dec 16, 2021
View reviewed changes
Copy link
Collaborator
@RCasatta RCasatta left a comment

Choose a reason for hiding this comment

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

ACK ab12410

@RCasatta RCasatta merged commit e5c6d65 into rust-bitcoin:master Dec 16, 2021
@dr-orlovsky dr-orlovsky mentioned this pull request Jan 15, 2022
Closed
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

@RCasatta RCasatta RCasatta approved these changes

Assignees
No one assigned
Labels
one ack PRs that have one ACK, so one more can progress them trivial Obvious, easy and quick to review (few lines or doc-only...)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add MAX_MONEY public constant to Amount
5 participants
@6293 @sanket1729 @Kixunil @dpc @RCasatta
0