Skip to content

feat/common/uint256_and_compact - #64

Merged
b-yap merged 4 commits into
masterfrom
feat/common/uint256_and_compact
Jan 17, 2022
Merged

feat/common/uint256_and_compact#64
b-yap merged 4 commits into
masterfrom
feat/common/uint256_and_compact

Conversation

@b-yap

@b-yap b-yap commented Jan 14, 2022

Copy link
Copy Markdown
Contributor

address comments about compact:
#62 (comment)
#62 (comment)

Because the # of files to review in my other PRs are getting larger, it's best to separate these in another PR.

Comment thread common/src/primitives/compact.rs Outdated

impl Compact {
// https://github.com/bitcoin/bitcoin/blob/7fcf53f7b4524572d1d0c9a5fdc388e87eb02416/src/arith_uint256.cpp#L203
pub fn into_uint256(self) -> Option<Uint256> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about try_from for this?

Comment thread common/src/util/endian.rs
@@ -0,0 +1,156 @@
// Rust Bitcoin Library

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like the util directory is exclusively used for the uint.rs stuff. Can we move these to their own directory under uint and expose them?

@TheQuantumPhysicist TheQuantumPhysicist left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Other than the few comments I made, it seems fine.

b-yap added 2 commits January 17, 2022 12:38
rename util directory to uint; move the uint file to the uint directory;
ran clippy.

for compact to uint256, `TryFrom` is implemented
@b-yap
b-yap merged commit 454e89f into master Jan 17, 2022
@b-yap
b-yap deleted the feat/common/uint256_and_compact branch January 17, 2022 11:10
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.

3 participants