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.

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

backport: trivial 2024 10 25 pr3 #6364

Merged

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Trivial backports

What was done?

How Has This Been Tested?

built locally

Breaking Changes

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)

@PastaPastaPasta PastaPastaPasta added this to the 22 milestone Oct 25, 2024
depends/Makefile Outdated Show resolved Hide resolved
src/test/fuzz/rpc.cpp Outdated Show resolved Hide resolved
src/net.h Outdated Show resolved Hide resolved
test/functional/test_framework/test_node.py Outdated Show resolved Hide resolved
contrib/guix/libexec/build.sh Outdated Show resolved Hide resolved
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-25-pr3 branch from fb02836 to 66566f3 Compare October 26, 2024 17:33
@PastaPastaPasta
Copy link
Member Author

dropped non-trivial commits

UdjinM6
UdjinM6 previously approved these changes Oct 26, 2024
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 66566f3 but also needs confirmation that Guix is ok (GH had and issue, couldn't reproduce it on my machine though)

kwvg
kwvg previously approved these changes Oct 26, 2024
Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

utACK 66566f3

@UdjinM6 UdjinM6 dismissed their stale review October 26, 2024 19:56

GH Guix is having issues again which is concerning. Most likely because of 27179.

@kwvg kwvg dismissed their stale review October 26, 2024 20:16

GitLab has reported errors (see TSan build) that are not routine

@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-25-pr3 branch from 66566f3 to d0e50ee Compare October 28, 2024 04:22
@UdjinM6
Copy link

UdjinM6 commented Oct 28, 2024

25677 needs a fix:

diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index e1418aa442..f15026cf7b 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -767,7 +767,7 @@ public:
     bool checkFinalTx(const CTransaction& tx) override
     {
         LOCK(cs_main);
-        return CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), tx);
+        return CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), tx);
     }
     bool isInstantSendLockedTx(const uint256& hash) override
     {

@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-25-pr3 branch from d0e50ee to eaaf0c8 Compare October 28, 2024 15:09
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-25-pr3 branch from eaaf0c8 to 867e34c Compare October 28, 2024 15:42
MarcoFalke and others added 9 commits October 29, 2024 13:01
…ollingBloom benchmark

fff9141 refactor: Remove deduplication of data in rollingbloom bench (phyBrackets)

Pull request description:

  Fixed up bitcoin#24088.

ACKs for top commit:
  vincenzopalazzo:
    ACK bitcoin@fff9141

Tree-SHA512: 9fef617bceb74a1aec4f4a1e7c4732c4764af3e8ac2fc02b84ce370e8b97431957ca17ee8f44fb96765f7304f8d7e5bfb951440db98ba40f240612f2232d215e
1075820 doc: Add gpg key import instructions for Windows (Dave Scotese)

Pull request description:

  This is a single commit to replace the three commits from bitcoin#23916

  I propose this change so that Windows users can more easily import signers' keys.

ACKs for top commit:
  sipsorcery:
    tACK 1075820.

Tree-SHA512: 7d4ec77ce10f751748c49f1453fa8baf0976b15af4f87dc27f4e2715ad73fbd7dc1f07fcf3e660d63a6b9eb895a5e4105774613d39a2328f73b92d9e6cff4ebd
…alidation function

fa86710 Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke)

Pull request description:

  It has been pointed out that a bug in this function can prevent block template creation. ( bitcoin#24080 (comment) ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b.

ACKs for top commit:
  ajtowns:
    ACK fa86710 - looks fine to me
  glozow:
    ACK fa86710

Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
9376a6d refactor: make active_chain_tip a reference (Aurèle Oulès)

Pull request description:

  This PR fixes a TODO introduced in bitcoin#21055.

  Makes `active_chain_tip` argument in `CheckFinalTxAtTip` function a reference instead of a pointer.

ACKs for top commit:
  dongcarl:
    ACK 9376a6d

Tree-SHA512: c36d1769e0b9598b7f79334704b26b73e958d54caa3bd7e4eff954f3964fcf3f5e3a44a5a760497afad51b76e1614c86314fe035e4083c855e3574a620de7f4d
007d6f0 test: fix `AddNode` unit test failure on OpenBSD (Sebastian Falbesoner)

Pull request description:

  On OpenBSD 7.4, the following check of the unit test `test_addnode_getaddednodeinfo_and_connection_detection` currently fails:
  ```
  BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true}));
  ```
  The reason for that is that this OS seemingly doesn't support the IPv4 shorthand notation with omitted zero-bytes:

  ```
  $ ping 127.1
  ping: no address associated with name
  ```

  As a simple fix, this PR skips the check for this with a pre-processor #if. On NetBSD and FreeBSD, `127.1` is resolved correctly to localhost and hence the test passes (thanks to vasild for verifying on the latter!).

ACKs for top commit:
  vasild:
    ACK 007d6f0

Tree-SHA512: 8ab8393c490e1ecc140e8ff74f6fa4d26d0dd77e6a77a241cd198314b8c5afee7422f95351ca05f4c1742433dab77016a8ccb8d28062f8edd4b703a918a2bbda
6e873df serfloat: improve/simplify tests (Pieter Wuille)
b45f1f5 serfloat: do not test encode(bits)=bits anymore (Pieter Wuille)

Pull request description:

  Closes bitcoin#28941.

  Our current tests for serfloat verify two distinct properties:
  1. Whether they roundtrip `double`->`uint64_t`->`double` (excluding NaN values) on all systems.
  2. Whether on systems with a typical floating point unit that encoding matches the hardware representation, as before v22.0, we would dump the hardware representation directly to disk and we wanted to retain compatibility with that.

  bitcoin#28941 seems to show that the second property doesn't always hold, but just for "subnormal" numbers (below $2^{-1021}$). Since we don't care about encoding these numbers, we could exclude such subnormal numbers from the hardware-identical representation test, but this PR goes further and just drops the second property entirely, as I don't think we care about edge-case compatibility with pre-v22.0 code for fee_estimates.dat (the only place it is used).

ACKs for top commit:
  glozow:
    ACK 6e873df
  fanquake:
    ACK 6e873df - It's not as much of a priority, but I think we could still backport this.

Tree-SHA512: e18ceee0753a7ee7e999fdfa10b014dc5bb67b6ef79522a0f8c76b889adcfa785772fc26ed7559bcb5a09a9938e243bb54eedd9549bc59080a2c8090155e2267
…lization

f65b0f6 index: Move last_locator_write_time and logging to end of threadsync loop (Fabian Jahr)

Pull request description:

  In the index sync thread, when initializing an index for the first time, stop callng BaseIndex::Commit when m_best_block_index is null, to avoid a spurious "failed to commit" error from that function. This error started happening in commit bitcoin@7878f97 from bitcoin#25494 and was reported by pstratem in bitcoin#26903 with an alternate fix.

ACKs for top commit:
  achow101:
    ACK f65b0f6
  ryanofsky:
    Code review ACK f65b0f6. Just moved log "Syncing" log line since last commit to avoid having to call now() twice.
  furszy:
    ACK f65b0f6
  TheCharlatan:
    ACK f65b0f6

Tree-SHA512: afa8f05786318d36346d167ff53ea0b3bc8abdb0ad04465d199dc3eb91e9f837369e24fcb7e24b5757b02d698ec504e61da6ac365eaf006c874fc07a424a7e20
b7e7e72 depends: fix mingw-w64 Qt DEBUG=1 build (fanquake)

Pull request description:

  The issue is that compilation is done with `x86_64-w64-mingw32-g++-posix`, but then linking is done with `x86_64-w64-mingw32-g++`.

  I'm guessing this has been broken since bitcoin#24131 (01d1845), but have not checked.

  Fixes bitcoin#29734.
  Unblocks bitcoin#29527 (`DEBUG=1` builds can be tested).

ACKs for top commit:
  hebasto:
    ACK b7e7e72, tested on Ubuntu 22.04 with the [installed](bitcoin#29734 (comment)) `g++-mingw-w64-x86-64` package.
  TheCharlatan:
    ACK b7e7e72

Tree-SHA512: 9e24e84046c0489c20971bb9c68d1a643c233837193c184f61bff79dfc8d7397a5c5526ac1a205ad423920f2589559cd01cb104ceb7f89515bb6421510d82ca9
…ilation

2de2ea2 build, depends: Fix `libmultiprocess` cross-compilation (Hennadii Stepanov)

Pull request description:

  On the master branch @ 3b12fc7, the following command fails:
  ```
  $ make -C depends libmultiprocess HOST=arm64-apple-darwin MULTIPROCESS=1
  ...
  [100%] Linking CXX executable mpgen
  ...
  clang++: error: linker command failed with exit code 1 (use -v to see invocation)
  ...
  ```

  This PR prevents building all default targets that include `mpgen`, which expectedly fails to link when cross-compiling.

ACKs for top commit:
  ryanofsky:
    Code review ACK 2de2ea2
  fanquake:
    ACK 2de2ea2 - I checked that this fixes the macOS cross-compilation issue. I'm assuming these packages are also likely to change further in the (near) future, given the changes going in upstream: https://github.com/chaincodelabs/libmultiprocess/pulls?q=is%3Apr+is%3Aclosed.

Tree-SHA512: 563551afbe483c923b52c6171f9d73bcc30bc4febd821b5abfe8aadb2ac601b94c2d10a73746ace3710d9f0afa4798eb090e77ccb1ae66a819495912802d91c9
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-25-pr3 branch from 867e34c to fbc0bce Compare October 29, 2024 18:01
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 fbc0bce

@PastaPastaPasta PastaPastaPasta merged commit 841ea27 into dashpay:develop Oct 29, 2024
33 checks passed
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.

6 participants