Bitcoin Core Github
42 subscribers
125K links
Download Telegram
🤔 mzumsande reviewed a pull request: "validation: set BLOCK_FAILED_CHILD correctly"
(https://github.com/bitcoin/bitcoin/pull/31835#pullrequestreview-2714280051)
Code Review ACK 17718660b8c95d1c12124ba2f38baf286a3bddf2
💬 hodlinator commented on issue "fuzz: psbt_base64_decode fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/32135#issuecomment-2751723855)
Been debugging this, and I suspect the issue is down to unspecified/differing evaluation order. I suspect MSVC is sometimes evaluating the RHS of the `==`-expression before the `LHS`.
```C++
assert(DecodeBase64PSBT(psbt, random_string, error) == error.empty());
```
Breaking out fixes the issue locally for me, probably by introducing a sequence-point.
```C++
bool success = DecodeBase64PSBT(psbt, random_string, error);
assert(success == error.empty());
```
💬 mzumsande commented on issue "RFC: Macro Regression Test Suite for Historical Reorgs":
(https://github.com/bitcoin/bitcoin/issues/32130#issuecomment-2751731539)
> since we can't have reorgs during IBD otherwise

I'd say we shouldn't really encounter reorgs during IBD in general - reorgs typically happen when the node is synced to the tip, a new node doing IBD today usually won't experience any of the historical reorgs.
Do I understand it correctly that what you suggest is to simulate a situation where the syncing node thinks it is out of IBD to replay these reorgs - by changing `-minchainwork` for the node under test to a lower value, and having some o
...
💬 yancyribbens commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2751781955)
> It might also be possible to generally have more precision if we update internally to replace CFeeRate with FeeFrac.

This is the most exciting option to me. Internally if FeeFrac is used, then I think that sets the stage to have an API that you can easily program to return either sat/vB, sat/kvB, sat/kwu by passing a param of your choosing at the time of making the call.
💬 ryanofsky commented on pull request "build: create Depends build type for depends and use it by default for depends builds":
(https://github.com/bitcoin/bitcoin/pull/31920#issuecomment-2751785951)
It might be nice to move the first commit 40a01e9d30b0c7deabd5996867f248637f3d1a08 into a separate PR because seems straightforwardly good (deduplicating code and moving flags that aren't host specific out of host-specific configurations) and it would remove complexity from this PR.

> Yeah, I don't think this is intended to address any of those concerns. This is intended to define a sane way of forwarding compile flags from depends.

Can you explain what specific problems or concerns this P
...
👍 pablomartin4btc approved a pull request: "refactor: Remove redundant and confusing calls to IsArgSet"
(https://github.com/bitcoin/bitcoin/pull/31896#pullrequestreview-2714365858)
re-ACK 0000fb3fd9f01f8a8cbe9de0bdc080023b6f00fc

No changes, just rebased (basically due to this #31278).
💬 luisschwab commented on pull request "wallet: make coinbase that will mature on the next block available for selection":
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2751818226)
What do you think @murchandamus?
💬 instagibbs commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2751840310)
re:propagation, do we not have the same race condition on most wallet made transactions due to the anti fee sniping logic?
👍 brunoerg approved a pull request: "contrib: document asmap-tool commands more thoroughly"
(https://github.com/bitcoin/bitcoin/pull/32110#pullrequestreview-2714421402)
ACK d85b980dfbd4eea7ed2968ab5cd0309576fbbcf9

happy to re-ack in case more things are addressed
👍 ryanofsky approved a pull request: "refactor: Remove redundant and confusing calls to IsArgSet"
(https://github.com/bitcoin/bitcoin/pull/31896#pullrequestreview-2714427001)
Code review ACK 0000fb3fd9f01f8a8cbe9de0bdc080023b6f00fc. No changes since last review other than rebase.
💬 maflcko commented on issue "fuzz: psbt_base64_decode fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/32135#issuecomment-2751872516)
@hodlinator Thanks! When the evaluation order isn't specified by the standard, it is unspecified. Mind submitting a fix?
💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2751880969)
Updated 0ec78961915da141b9c68a39ec0ebd5091c13be0 -> fae300f159cd25a12abf4d5fbb93135cececd38d ([`pr/mine.21`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.21) -> [`pr/mine.22`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.22), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.21..pr/mine.22)) just tweaking comments
💬 hebasto commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2751896236)
My Guix build:
```
aarch64
02c610ee1471cc2671588a999e5ef914547cd55352ddc084faf7a9a55d79098b guix-build-963355037fe7/output/aarch64-linux-gnu/SHA256SUMS.part
11c1779525e0ebfbb55badd50c818cc02ac85a113077cc6074e6d11d4bd98677 guix-build-963355037fe7/output/aarch64-linux-gnu/bitcoin-963355037fe7-aarch64-linux-gnu-debug.tar.gz
5ef64bbfadca863a09fada8da1b6cf4543c318bb4208f67c7473718c5dec6f55 guix-build-963355037fe7/output/aarch64-linux-gnu/bitcoin-963355037fe7-aarch64-linux-gnu.tar.gz
80beeb5b
...
👍 brunoerg approved a pull request: "contrib: document asmap-tool commands more thoroughly"
(https://github.com/bitcoin/bitcoin/pull/32110#pullrequestreview-2714487561)
reACK 7ed3df06f7d0c18ef070bd04bdae3b5ddacd52fd

d85b980dfbd4eea7ed2968ab5cd0309576fbbcf9..7ed3df06f7d0c18ef070bd04bdae3b5ddacd52fd only adds an information about pypy3
💬 jurraca commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2751910412)
added a note about `python3` vs `pypy3` at the top, and also in the `diff`section [aff84d1](https://github.com/bitcoin/bitcoin/pull/32110/commits/aff84d1c89f5052e69f4fe03306bb094be13eb01).
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r2012564819)
noted! I will remove it anyway when removing the rpc call
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2751988619)
ACK 7ed3df06f7d0c18ef070bd04bdae3b5ddacd52fd
💬 maflcko commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r2012578881)
It is going to be a few months until 30.x branch-off, so it could make sense to fix it earlier.

Maybe along with the logging suggestion from https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1998876768?

Could have:

```py
self.log.info("Tests for deprecated RPC methods (if any)")

if self.is_wallet_compiled():
self.log.info("Tests for deprecated wallet-related RPC methods (if any)")

# Move the tests from this pull here
```
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r2012585405)
Make sense, will do. One question, as this is already merged, should this change be squashed with the other commits, should they be added on top of them or should be commited in another PR?
💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2752009943)
re-ACK fae300f159cd25a12abf4d5fbb93135cececd38d