Skip to content

Conversation

@PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Sep 28, 2023

Issue being fixed or feature implemented

A few small backports

What was done?

backports

How Has This Been Tested?

Building

Breaking Changes

None

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@github-actions
Copy link

github-actions bot commented Oct 5, 2023

This pull request has conflicts, please rebase.

@PastaPastaPasta PastaPastaPasta force-pushed the backport-uint256-modern branch from 3e82b70 to e1608ac Compare October 5, 2023 13:24
@PastaPastaPasta PastaPastaPasta marked this pull request as ready for review October 5, 2023 13:25
@PastaPastaPasta PastaPastaPasta added this to the 20 milestone Oct 5, 2023
@PastaPastaPasta PastaPastaPasta force-pushed the backport-uint256-modern branch from e1608ac to ad4a9a2 Compare October 28, 2023 01:01
@UdjinM6 UdjinM6 changed the title Backport uint256 modern backport: uint256 modern Oct 28, 2023
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 changed the title backport: uint256 modern backport: merge bitcoin#26105, #26345 (uint256 modern) Oct 28, 2023
fanquake and others added 3 commits October 30, 2023 09:50
…plicating logic

04fee75 Use ReadLE64 in uint256::GetUint64() instead of duplicating logic (Pieter Wuille)

Pull request description:

  No need to have a (naive) copy of the `ReadLE64` logic inside `uint256::GetUint64`, when we have an optimized function for exactly that.

ACKs for top commit:
  davidgumberg:
    ACK 04fee75
  jonatack:
    ACK 04fee75 review, this use of ReadLE64() is similar to the existing invocation by Num3072::Num3072(), sanity checked that before and after this change GetUint64() returns the same result (debug build, clang 13)

Tree-SHA512: 0fc2681536a18d82408411bcc6d5c6445fb96793fa43ff4021cd2933d46514c725318da35884f428d1799023921f33f8af091ef428ceb96a50866ac53a345356
935acdc refactor: modernize the implementation of uint256.* (pasta)

Pull request description:

  - Constructors of uint256 to utilize Span instead of requiring a std::vector
  - converts m_data into a std::array
  - Prefers using `WIDTH` instead of `sizeof(m_data)`
  - make all the things constexpr
  - replace C style functions with c++ equivalents
      - memset -> std::fill
          This may also be replaced by std::memset, but I think that std::fill is more idiomatic of modern c++ and readable.
      - memcpy -> std::copy
          Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy)
          This could also likely be replaced by std::memcpy, but as said above, I believe the using std::copy is the more c++ way to do anything and is almost guaranteed to compile to the same asm
      - memcmp -> std::memcmp

ACKs for top commit:
  achow101:
    ACK 935acdc
  hebasto:
    Approach ACK 935acdc.
  aureleoules:
    reACK 935acdc
  john-moffett:
    ACK 935acdc
  stickies-v:
    Approach ACK 935acdc

Tree-SHA512: 4f1ba54ff2198eea0e505d41e73d552c84c60f6878d5c85a94a8ab57f39afc94ef8d79258e7afd01fa84ec2a99f4404bb877eecd671f65e1ee9273f3129fc650
@PastaPastaPasta PastaPastaPasta force-pushed the backport-uint256-modern branch from ad4a9a2 to a2c3031 Compare October 30, 2023 14:50
@PastaPastaPasta PastaPastaPasta merged commit 7440078 into dashpay:develop Oct 30, 2023
@PastaPastaPasta PastaPastaPasta deleted the backport-uint256-modern branch October 30, 2023 14:50
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