👍 instagibbs approved a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2004196371)
ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2004196371)
ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197
⚠️ Bolits95 opened an issue: "Pago"
(https://github.com/bitcoin/bitcoin/issues/29894)
ethereum:0x249cA82617eC3DfB2589c4c17ab7EC9765350a18@1/transfer?address=0x8FBA23961A2DeE9628366d3Bc91574Bdad4d9740
(https://github.com/bitcoin/bitcoin/issues/29894)
ethereum:0x249cA82617eC3DfB2589c4c17ab7EC9765350a18@1/transfer?address=0x8FBA23961A2DeE9628366d3Bc91574Bdad4d9740
✅ fanquake closed an issue: "Pago"
(https://github.com/bitcoin/bitcoin/issues/29894)
(https://github.com/bitcoin/bitcoin/issues/29894)
:lock: fanquake locked an issue: "Pago"
(https://github.com/bitcoin/bitcoin/issues/29894)
(https://github.com/bitcoin/bitcoin/issues/29894)
📝 fanquake opened a pull request: "[RFC] guix: remove xz from deps"
(https://github.com/bitcoin/bitcoin/pull/29895)
This isn't (easily) doable:
* Qt only officially ships `.tar.xz` or `.zip`. However it's `.zip` artifacts seem to have been generated on a Windows machine? Which means they are polluted with Windows line endings/similar, which break all of our patches. This could possibly be worked around, but is messy / annoying. The commit here is WIP in that it needs extra handling for all of the tag tarballs being named the same thing. (You can Guix build with `NO_QT=1` to test).
* libxkbcommon has no alt
...
(https://github.com/bitcoin/bitcoin/pull/29895)
This isn't (easily) doable:
* Qt only officially ships `.tar.xz` or `.zip`. However it's `.zip` artifacts seem to have been generated on a Windows machine? Which means they are polluted with Windows line endings/similar, which break all of our patches. This could possibly be worked around, but is messy / annoying. The commit here is WIP in that it needs extra handling for all of the tag tarballs being named the same thing. (You can Guix build with `NO_QT=1` to test).
* libxkbcommon has no alt
...
⚠️ maflcko opened an issue: "Intermittent issue in p2p_handshake.py", line 75, in run_test self.test_desirable_service_flags(node, [NODE_NETWORK | NODE_WITNESS], not true after 2400.0 seconds"
(https://github.com/bitcoin/bitcoin/issues/29896)
https://cirrus-ci.com/task/5875023322284032?logs=ci#L3873
```
test 2024-04-16T12:25:11.924000Z TestFramework.p2p (DEBUG): Received message from 0:0: msg_getheaders(locator=CBlockLocator(vHave=[25904390216175313194312162455780078753009692101200952252992871177711616225820, 10439194800033568075353954394040901046437382900627609508673849620975391395575, 40446225147528968444488730747006358517695899943732003474923480031726915305221, 3288163205360417724924785454586033509877330136421489177587429566
...
(https://github.com/bitcoin/bitcoin/issues/29896)
https://cirrus-ci.com/task/5875023322284032?logs=ci#L3873
```
test 2024-04-16T12:25:11.924000Z TestFramework.p2p (DEBUG): Received message from 0:0: msg_getheaders(locator=CBlockLocator(vHave=[25904390216175313194312162455780078753009692101200952252992871177711616225820, 10439194800033568075353954394040901046437382900627609508673849620975391395575, 40446225147528968444488730747006358517695899943732003474923480031726915305221, 3288163205360417724924785454586033509877330136421489177587429566
...
💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2059591077)
This seems like an improvement, but I think the original suggestion of just adding a `std::error& error` output parameter to the CTxMemPool constructor (or a `util::Result<void>& result` output parameter) would provide a less rigid API than making the CTxMemPool constructor private and requiring every instance to be allocated on the heap and owned by a unique_ptr. This would also allow keeping the fuzz test behavior unchanged. The new fuzz test behavior is probably better, but maybe fuzz testing
...
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2059591077)
This seems like an improvement, but I think the original suggestion of just adding a `std::error& error` output parameter to the CTxMemPool constructor (or a `util::Result<void>& result` output parameter) would provide a less rigid API than making the CTxMemPool constructor private and requiring every instance to be allocated on the heap and owned by a unique_ptr. This would also allow keeping the fuzz test behavior unchanged. The new fuzz test behavior is probably better, but maybe fuzz testing
...
💬 laanwj commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1567731932)
Would it be possible to change these to values instead of defined/undefined? It seems to me that changing these to `ifdef` makes it less safe because you lose the protection added by the warning in the first place. (say, when you forget to include bitcoin-config)
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1567731932)
Would it be possible to change these to values instead of defined/undefined? It seems to me that changing these to `ifdef` makes it less safe because you lose the protection added by the warning in the first place. (say, when you forget to include bitcoin-config)
👍 alfonsoromanz approved a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2004262314)
Tested ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2004262314)
Tested ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197
⚠️ maflcko opened an issue: "Intermittent issue in p2p_unrequested_blocks.py", line 98, in run_test test_node.send_and_ping(msg_block(blocks_h2[0])) assert self.is_connected AssertionError / AcceptBlock FAILED (time-too-old, block's timestamp is too early) "
(https://github.com/bitcoin/bitcoin/issues/29897)
https://github.com/bitcoin/bitcoin/actions/runs/8689751788/job/23828088498?pr=29877#step:27:584
```
node0 2024-04-15T13:27:36.008782Z [D:\a\bitcoin\bitcoin\src\net_processing.cpp:4743] [ProcessMessage] [net] received block 20076ff9e9d9bace59d7621747b34dfb4af9b2f75ff499017397854f9fefb930 peer=0
node0 2024-04-15T13:27:36.008822Z [D:\a\bitcoin\bitcoin\src\validation.cpp:4145] [AcceptBlockHeader] [validation] AcceptBlockHeader: Consensus::ContextualCheckBlockHeader: 20076ff9e9d9bace59d7621747
...
(https://github.com/bitcoin/bitcoin/issues/29897)
https://github.com/bitcoin/bitcoin/actions/runs/8689751788/job/23828088498?pr=29877#step:27:584
```
node0 2024-04-15T13:27:36.008782Z [D:\a\bitcoin\bitcoin\src\net_processing.cpp:4743] [ProcessMessage] [net] received block 20076ff9e9d9bace59d7621747b34dfb4af9b2f75ff499017397854f9fefb930 peer=0
node0 2024-04-15T13:27:36.008822Z [D:\a\bitcoin\bitcoin\src\validation.cpp:4145] [AcceptBlockHeader] [validation] AcceptBlockHeader: Consensus::ContextualCheckBlockHeader: 20076ff9e9d9bace59d7621747
...
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567724841)
would be good to explicitly test that the parent-giver isn't punished in this scenario, and test if parent is consensus-bad it results in something expected.
```
diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py
index 603cbf08a9..1c887289dc 100755
--- a/test/functional/p2p_opportunistic_1p1c.py
+++ b/test/functional/p2p_opportunistic_1p1c.py
@@ -164,78 +164,129 @@ class PackageRelayTest(BitcoinTestFramework):
# itself and with a
...
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567724841)
would be good to explicitly test that the parent-giver isn't punished in this scenario, and test if parent is consensus-bad it results in something expected.
```
diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py
index 603cbf08a9..1c887289dc 100755
--- a/test/functional/p2p_opportunistic_1p1c.py
+++ b/test/functional/p2p_opportunistic_1p1c.py
@@ -164,78 +164,129 @@ class PackageRelayTest(BitcoinTestFramework):
# itself and with a
...
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567718964)
```Suggestion
# 3. Sender relays the parent. Parent+Child are evaluated as a package and rejected.
```
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567718964)
```Suggestion
# 3. Sender relays the parent. Parent+Child are evaluated as a package and rejected.
```
💬 pinheadmz commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2059641989)
Rebasing on master since one commit was merged separately in #29649
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2059641989)
Rebasing on master since one commit was merged separately in #29649
💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2059650726)
Thanks for the update! I noticed you didn't include Ocean. While they aren't of significant hash power, they do in fact do full-rbf for some of their hash power as they give miners an option of picking Knots (full-rbf) or Core (defaults) templates.
Also, Wasabi is now relying on full-rbf in production: https://github.com/zkSNACKs/WalletWasabi/pull/12677
tl;dr: their coordinator assumes that if a double spend is detected of lower fee-rate, the coinjoin will most likely win anyway regardless of
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2059650726)
Thanks for the update! I noticed you didn't include Ocean. While they aren't of significant hash power, they do in fact do full-rbf for some of their hash power as they give miners an option of picking Knots (full-rbf) or Core (defaults) templates.
Also, Wasabi is now relying on full-rbf in production: https://github.com/zkSNACKs/WalletWasabi/pull/12677
tl;dr: their coordinator assumes that if a double spend is detected of lower fee-rate, the coinjoin will most likely win anyway regardless of
...
✅ maflcko closed an issue: "Memory leak via API listtransactions"
(https://github.com/bitcoin/bitcoin/issues/25229)
(https://github.com/bitcoin/bitcoin/issues/25229)
💬 maflcko commented on issue "Memory leak via API listtransactions":
(https://github.com/bitcoin/bitcoin/issues/25229#issuecomment-2059651519)
I don't think there is anything that can be done here. The final JSON RPC reply fully written to a single byte buffer must be stored in memory contiguously. If such an amount of memory is not available, it needs to be allocated.
In theory it could be returned, for example by `malloc_trim`, but what would be the point, if on the next RPC call it will need to be allocated again?
(https://github.com/bitcoin/bitcoin/issues/25229#issuecomment-2059651519)
I don't think there is anything that can be done here. The final JSON RPC reply fully written to a single byte buffer must be stored in memory contiguously. If such an amount of memory is not available, it needs to be allocated.
In theory it could be returned, for example by `malloc_trim`, but what would be the point, if on the next RPC call it will need to be allocated again?
💬 maflcko commented on issue "Memory leak via API listtransactions":
(https://github.com/bitcoin/bitcoin/issues/25229#issuecomment-2059656053)
If you are looking for a workaround, you can either provide more memory (or swap), or you could try to call the RPC with smaller `count` values.
(https://github.com/bitcoin/bitcoin/issues/25229#issuecomment-2059656053)
If you are looking for a workaround, you can either provide more memory (or swap), or you could try to call the RPC with smaller `count` values.
💬 achow101 commented on pull request "doc: Add example of mixing private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/28373#issuecomment-2059662567)
ACK 24b67fa9f602cdeac0e9736256f77d048f616c48
(https://github.com/bitcoin/bitcoin/pull/28373#issuecomment-2059662567)
ACK 24b67fa9f602cdeac0e9736256f77d048f616c48
💬 achow101 commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1566472098)
In 000000000c5fad4aca360a6e914c7779167838db "Add bech32 error short & multiple separators cases"
This is incorrect. `1` is an allowed character in the human readable part, hence a multiple separators check doesn't make sense. The real separator is simply the last `1` that appears. See https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#bech32
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1566472098)
In 000000000c5fad4aca360a6e914c7779167838db "Add bech32 error short & multiple separators cases"
This is incorrect. `1` is an allowed character in the human readable part, hence a multiple separators check doesn't make sense. The real separator is simply the last `1` that appears. See https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#bech32
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567763258)
suggestion:
call this `med_fee_child` and use the mempoolminfee directly since that's a more meaningful value than `2*FEERATE_1SAT_VB` which is below the floating minfee
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567763258)
suggestion:
call this `med_fee_child` and use the mempoolminfee directly since that's a more meaningful value than `2*FEERATE_1SAT_VB` which is below the floating minfee