💬 fanquake commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1183002436)
This is against master (binaries produced with a Guix build). The symbol/security scripts are (only) expected to work with the version of the source they are shipped with. In this case, we are using a newer version of glibc to produce the binaries in master, hence the difference. Think I also alluded to this in a comment above.
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1183002436)
This is against master (binaries produced with a Guix build). The symbol/security scripts are (only) expected to work with the version of the source they are shipped with. In this case, we are using a newer version of glibc to produce the binaries in master, hence the difference. Think I also alluded to this in a comment above.
💬 TheCharlatan commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1183005546)
Ah right, all the better that we catch these now. I found a recent guix build and it checks out there.
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1183005546)
Ah right, all the better that we catch these now. I found a recent guix build and it checks out there.
👍 TheCharlatan approved a pull request: "contrib: add ELF OS ABI check to symbol-check.py"
(https://github.com/bitcoin/bitcoin/pull/26953#pullrequestreview-1409819053)
ACK 65ba8a79a2919a0bd89f2f2d981e072d4f2f549d
(https://github.com/bitcoin/bitcoin/pull/26953#pullrequestreview-1409819053)
ACK 65ba8a79a2919a0bd89f2f2d981e072d4f2f549d
👍 theStack approved a pull request: "test: Simplify feature_fastprune.py"
(https://github.com/bitcoin/bitcoin/pull/27553#pullrequestreview-1409826743)
ACK fa17767154e21e9ed00782a9e4cf9a3d1d66f5d1
Checked that the size of the generated blocks in master and PR are about equal by adding
`print(len(bytes.fromhex(self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 0))))`
at the end of the test (interestingly, in the PR the block is exactly 1 byte larger [65904 vs. 65903] but that doesn't matter of course).
(https://github.com/bitcoin/bitcoin/pull/27553#pullrequestreview-1409826743)
ACK fa17767154e21e9ed00782a9e4cf9a3d1d66f5d1
Checked that the size of the generated blocks in master and PR are about equal by adding
`print(len(bytes.fromhex(self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 0))))`
at the end of the test (interestingly, in the PR the block is exactly 1 byte larger [65904 vs. 65903] but that doesn't matter of course).
📝 0xB10C opened a pull request: "doc: clarify processing of mempool-msgs when NODE_BLOOM"
(https://github.com/bitcoin/bitcoin/pull/27559)
Under which circumstances we process received 'mempool' P2P messages caused confusion in #27426. Rather than bike-shedding the formulation of the IF-statement, this adds a comment clarifying when we process the message. Also, correcting the `m_send_mempool` description.
(https://github.com/bitcoin/bitcoin/pull/27559)
Under which circumstances we process received 'mempool' P2P messages caused confusion in #27426. Rather than bike-shedding the formulation of the IF-statement, this adds a comment clarifying when we process the message. Also, correcting the `m_send_mempool` description.
💬 furszy commented on pull request "wallet: bugfix, 'wallet_load_ckey' unit test fails with bdb":
(https://github.com/bitcoin/bitcoin/pull/26644#issuecomment-1532254497)
> Would it be easier to do this on top of #26715?
Was waiting for #27556 CI to close this in favor of #26715 actually. The heart of this PR is 32ffea2 which is just a test code re-ordering to by-pass the memory-only db limitations in the db rewrite + environment reload events executed in the wallet encryption process (we don't care about what db engine the test uses, we test exercises loading invalid data into the wallet).
With #26715 inclusion, the test db is just a raw map in-memory and
...
(https://github.com/bitcoin/bitcoin/pull/26644#issuecomment-1532254497)
> Would it be easier to do this on top of #26715?
Was waiting for #27556 CI to close this in favor of #26715 actually. The heart of this PR is 32ffea2 which is just a test code re-ordering to by-pass the memory-only db limitations in the db rewrite + environment reload events executed in the wallet encryption process (we don't care about what db engine the test uses, we test exercises loading invalid data into the wallet).
With #26715 inclusion, the test db is just a raw map in-memory and
...
✅ furszy closed a pull request: "wallet: bugfix, 'wallet_load_ckey' unit test fails with bdb"
(https://github.com/bitcoin/bitcoin/pull/26644)
(https://github.com/bitcoin/bitcoin/pull/26644)
💬 litch commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1532325830)
tACK - we're running this fork in several contexts and it works as expected
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1532325830)
tACK - we're running this fork in several contexts and it works as expected
💬 ajtowns commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#discussion_r1183227419)
As a datapoint, importing my mempool now (600MB used, 237k entries) seems to take about 25 minutes. Not a particularly fast machine (though not an rpi either), debug build, etc, so something of a worst-case scenario, but still.
(https://github.com/bitcoin/bitcoin/pull/27476#discussion_r1183227419)
As a datapoint, importing my mempool now (600MB used, 237k entries) seems to take about 25 minutes. Not a particularly fast machine (though not an rpi either), debug build, etc, so something of a worst-case scenario, but still.
💬 fanquake commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1532639812)
> or an issue with the LIEF stubs. Am following up.
This has now been resolved upstream in LIEF. See https://github.com/lief-project/LIEF/issues/909 & https://github.com/lief-project/LIEF/commit/2e06bdb4af4586d1f5dcb9e9ccb027b55d457e24.
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1532639812)
> or an issue with the LIEF stubs. Am following up.
This has now been resolved upstream in LIEF. See https://github.com/lief-project/LIEF/issues/909 & https://github.com/lief-project/LIEF/commit/2e06bdb4af4586d1f5dcb9e9ccb027b55d457e24.
💬 fanquake commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#issuecomment-1532649068)
Guix Build:
```bash
80e0201c2f4c0c2639bfa4f53d7a0ca982e42e19e7abcbd013675e8323394bd8 guix-build-65ba8a79a291/output/aarch64-linux-gnu/SHA256SUMS.part
c19b6dd1413d1796d59cee5243b8b004460e36ff0f86f277febbdcc43e5c5356 guix-build-65ba8a79a291/output/aarch64-linux-gnu/bitcoin-65ba8a79a291-aarch64-linux-gnu-debug.tar.gz
9a6f9a798d1d686828e1c658735bc230e4d141b6abfb75e6e819058566707535 guix-build-65ba8a79a291/output/aarch64-linux-gnu/bitcoin-65ba8a79a291-aarch64-linux-gnu.tar.gz
20b76fd565b64c01
...
(https://github.com/bitcoin/bitcoin/pull/26953#issuecomment-1532649068)
Guix Build:
```bash
80e0201c2f4c0c2639bfa4f53d7a0ca982e42e19e7abcbd013675e8323394bd8 guix-build-65ba8a79a291/output/aarch64-linux-gnu/SHA256SUMS.part
c19b6dd1413d1796d59cee5243b8b004460e36ff0f86f277febbdcc43e5c5356 guix-build-65ba8a79a291/output/aarch64-linux-gnu/bitcoin-65ba8a79a291-aarch64-linux-gnu-debug.tar.gz
9a6f9a798d1d686828e1c658735bc230e4d141b6abfb75e6e819058566707535 guix-build-65ba8a79a291/output/aarch64-linux-gnu/bitcoin-65ba8a79a291-aarch64-linux-gnu.tar.gz
20b76fd565b64c01
...
💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1183400073)
Because the current codebase only support P2WSH, there is no support for legacy P2SH.
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1183400073)
Because the current codebase only support P2WSH, there is no support for legacy P2SH.
🚀 fanquake merged a pull request: "test: Simplify feature_fastprune.py"
(https://github.com/bitcoin/bitcoin/pull/27553)
(https://github.com/bitcoin/bitcoin/pull/27553)
💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1183405185)
What's the rationale for changing this? Do whitespaces prevent the analysis? Happy to do this (and the other similar comments) but i'd rather not go through the hassle of applying it on every single of the 21 commits and solve the rebase conflicts if it's just a style nit. Let me know!
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1183405185)
What's the rationale for changing this? Do whitespaces prevent the analysis? Happy to do this (and the other similar comments) but i'd rather not go through the hassle of applying it on every single of the 21 commits and solve the rebase conflicts if it's just a style nit. Let me know!
⚠️ Canvict opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/27560)
(https://github.com/bitcoin/bitcoin/issues/27560)
✅ fanquake closed an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/27560)
(https://github.com/bitcoin/bitcoin/issues/27560)
:lock: fanquake locked an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/27560)
(https://github.com/bitcoin/bitcoin/issues/27560)
💬 fanquake commented on pull request "build: include example bitcoin.conf in build outputs under share/examples/ subdirectory":
(https://github.com/bitcoin/bitcoin/pull/25935#issuecomment-1532665922)
Given the above, going to close this for now.
(https://github.com/bitcoin/bitcoin/pull/25935#issuecomment-1532665922)
Given the above, going to close this for now.
✅ fanquake closed a pull request: "build: include example bitcoin.conf in build outputs under share/examples/ subdirectory"
(https://github.com/bitcoin/bitcoin/pull/25935)
(https://github.com/bitcoin/bitcoin/pull/25935)
💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1183410413)
This specific case is the very common match-all-variants-and-assert pattern so i think it's good like that. It's also documented this way in our style guidelines: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures.
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1183410413)
This specific case is the very common match-all-variants-and-assert pattern so i think it's good like that. It's also documented this way in our style guidelines: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures.