Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 Sjors commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1723150550)
Updated the comment to reflect that.
👍 tdb3 approved a pull request: "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate"
(https://github.com/bitcoin/bitcoin/pull/30636#pullrequestreview-2247731072)
ACK 917e70a6206c62c4c492fa922425fc8e00d3f328

These tests are great additions.
Changes incorporate comments nicely.
Ran functionals locally (passed).
🤔 fjahr reviewed a pull request: "wallet: Ensure best block matches wallet scan state"
(https://github.com/bitcoin/bitcoin/pull/30221#pullrequestreview-2247713859)
Concept ACK

I can confirm that best block locator and `last_processed_block` being different is confusing, see also https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1719951882

Currently, this needs a rebase and I'm curious if @achow101 plans to make further changes based on @ryanofsky 's comments.
💬 fjahr commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r1723141952)
I'm thinking that it might make sense to keep this backup where it is and add second backup created at 199. Then this could test that both cases work as expected, i.e. 199 errors below and 299 doesn't. 299 is an interesting edge case in general (snapshot height == backup height).
💬 paplorinc commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723158537)
> I think adding "missing" space can be done in the rename commit.

Agree, would have preferred it that way as well.
But to be fair, git can ignore whitespaces during blame: https://git-scm.com/docs/git-blame#Documentation/git-blame.txt--w
💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723168808)
Started changing it based on your comment, but as there is already another if-return false above in the untouched code, I think it's more consistent to keep as-is.
💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723169146)
Thanks, taking that!
💬 paplorinc commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2298662545)
reACK c3fa29db1b05aa51f74cc8be5bdae59be4f3b7c0

https://github.com/bitcoin/bitcoin/compare/80a596ecca5df9f471d9fbcd9fcd15ddd296cdca..c3fa29db1b05aa51f74cc8be5bdae59be4f3b7c0:
* Removed many `<uint8_t>` inside `ToScript`
* out_exp -> string_view
* braces in wallet_crypto_tests.cpp and crypter.cpp
* remove_reference -> remove_reference_t
📝 brunoerg opened a pull request: "test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py`"
(https://github.com/bitcoin/bitcoin/pull/30683)
Sending a `verack` without `wtxidrelay`, regardless of `sendtxrcncl`, will trigger `Forget txreconciliation state of peer`. To ensure that we are testing that `SENDTXRCNCL` without `WTXIDRELAY` is ignored, we can first verify that the peer was registered.
💬 MummaStacks commented on issue "Running Bitcoin Bitcore on new Apple M3":
(https://github.com/bitcoin/bitcoin/issues/30680#issuecomment-2298710905)
thank you it does appear to be :) sorry for doubling up
💬 fanquake commented on issue "Control-flow application capabilities for `x86_64-linux-gnu` release binaries":
(https://github.com/bitcoin/bitcoin/issues/30677#issuecomment-2298711926)
> Should we adjust glibc [Hardware Capability Tunables](https://www.gnu.org/software/libc/manual/html_node/Hardware-Capability-Tunables.html)?

Why?
💬 brunoerg commented on pull request "test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py`":
(https://github.com/bitcoin/bitcoin/pull/30683#issuecomment-2298714161)
Can be tested with:
```diff
diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py
index 2c7216b5ca..1e0e82f076 100755
--- a/test/functional/p2p_sendtxrcncl.py
+++ b/test/functional/p2p_sendtxrcncl.py
@@ -218,7 +218,6 @@ class SendTxRcnclTest(BitcoinTestFramework):
self.log.info('SENDTXRCNCL without WTXIDRELAY is ignored (recon state is erased after VERACK)')
peer = self.nodes[0].add_p2p_connection(PeerNoVerack(wtxidrelay=False), send_versi
...
💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2298715703)
re-ACK c3fa29db1b05aa51f74cc8be5bdae59be4f3b7c0
MummaStacks closed an issue: "Running Bitcoin Bitcore on new Apple M3"
(https://github.com/bitcoin/bitcoin/issues/30680)
fanquake closed an issue: "Unable to build Bitcoin using './contrib/guix/guix-build'"
(https://github.com/bitcoin/bitcoin/issues/30676)
💬 brunoerg commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1723269955)
In 9d84717c92ed052dea6f78c715833d328835ba79 "fuzz: Add harness for p2p headers sync": Is there any reason to use `resize` instead of simply doing:
```diff
diff --git a/src/test/fuzz/p2p_headers_sync.cpp b/src/test/fuzz/p2p_headers_sync.cpp
index 8de39431b8..f474263620 100644
--- a/src/test/fuzz/p2p_headers_sync.cpp
+++ b/src/test/fuzz/p2p_headers_sync.cpp
@@ -48,10 +48,11 @@ void HeadersSyncSetup::ResetAndInitialize()
connman.StopNodes();

NodeId id{0};
- std::vector<Conn
...
💬 hodlinator commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1723257479)

nit in cb9d4712aec195f1fc5b3bc0a46bc13b46477b85:
```suggestion
CCoinsViewTest(FastRandomContext& rng LIFETIMEBOUND) : m_rng{rng} {}
```
🤔 hodlinator reviewed a pull request: "test: [refactor] Use m_rng directly"
(https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2243672559)
Concept ACK 164732369fc5ecf4c7705136a2de884419023b5a
💬 hodlinator commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1720433250)
It would leave the code in a better state after this PR if the constructor wasn't made into this odd sidecar lambda. We already have `m_rng` available from prior commit so just pass it down.

Applies on top of and reverts part of ac4e757bc3633e01db973201e1633331c0815e3a "Give unit test functions access to test state":
```diff
diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp
index d2b28b9b18..ac4fe4404e 100644
--- a/src/test/cuckoocache_tests.cpp
+++ b/src/test/
...
👍 theStack approved a pull request: "test: check xor.dat creation when missing"
(https://github.com/bitcoin/bitcoin/pull/30669#pullrequestreview-2248062503)
ACK 7b7c287a33dc0c6f09bf32f31a8f05941b0cc9a2

Thanks for following up! If you want, you could also tackle the suggestion to move the `read_xor_key` helper to the `TestNode` class, see https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1717985952