💬 maflcko commented on issue "test: intermittent issue in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/issues/31946#issuecomment-2740929430)
Yeah, let's continue review/discussion in one place ( #32103)

💬 luisschwab 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-2740940036)
Shouldn't these concerns be separated (transaction building and transaction broadcasting)?

💬 maflcko commented on pull request "ci: run test-each-commit on merge to master":
(https://github.com/bitcoin/bitcoin/pull/32103#issuecomment-2740984496)
> I think what you want is to checkout each old commit and then merge it (inside the CI task) with master. Not sure how to achieve it otherwise. Maybe you can share your steps to reproduce with `git`?

To clarify, my suggestion would be to modify the `git rebase --exec 'run_test' base` into `git rebase --exec 'git log -1 && git merge --no-commit master && echo run_test && git merge --abort' base`. However, I haven't tested this.

💬 maflcko 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-2740994889)
The wallet generally is a bit conservative when it comes to creating transactions that may not propagate well. (For example, it doesn't yet offer fullrbf bumps). Of course, anyone is free to manually create the wanted transaction early and propagate it manually.

💬 maflcko commented on pull request "refactor: Simplifies ProcessMessage for NetMsgType::TX":
(https://github.com/bitcoin/bitcoin/pull/32104#discussion_r2006005124)
Yes. Alternatively, at a minimum the `txid` and `wtxid` types should be adjusted here. Otherwise, this refactor may cause a silent merge conflict with the type-safety refactor.

👍 vasild approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2703458243)
ACK 418236c106e32abd7357551d309f8e6d1e494f72

💬 l0rinc commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2006064197)
> This could be an assume

I'm fine with both, thanks, changed reader & writer.

> This could be an assume, given that the program can continue normally, albeit it is a pessimization or possible OOM on low memory machines?

You mean reading the whole block into memory could be the last straw?
Can you please detail the pessimization scenario?

👍 vasild approved a pull request: "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods"
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2703488792)
ACK cc1001f3bf17b31512c05fb359e09483a07fb2a3

💬 ryanofsky commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2741144182)
re: https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2738349197

Thanks, if you don't think a separate repository for C bindings would be good, or worth the tradeoffs, that's fine. I was just excited about the idea because I realized with cmake config modules it would be easy to implement technically, and it seemed like a natural starting point to experiment with splitting the project up into different repositories while being able to share utilities and infrastructure.

Just to ex
...

💬 luisschwab 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-2741163824)
I understand that, but I think the wallet should strive for correctness, given we're dealing with coinbase UTXOs which only knowledgeable people have access to (miners and developers). This is an edge case regular users won't ever experience, myself included.

💬 maflcko commented on pull request "refactor: Enforce readability-avoid-const-params-in-decls":
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-2741178844)
rebased

💬 sipa 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-2741178910)
@luisschwab Well it's not incorrect to be conservative. And if regular users don't experience it, why does it matter at all?

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2741184656)
`53d2c8e0e2...c1c6a056a0`: rebase due to conflicts

💬 jonatack commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2741185074)
ACK c6eca6f3961d780d6a395c3346e44be1a0072f08

💬 jonatack commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2741194008)
(Meta: the current GitHub safetyism and friction is annoying, when all you want to do is ACK 😄)

**"This issue is highly active. Reconsider commenting unless you have read all the comments and have something to add."**

**"Your link may be misinterpreted by GitHub."**

💬 luisschwab 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-2741201839)
I don't think not being able to select an otherwise spendable UTXO is correct.

💬 maflcko commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2006146926)
I just mean that the assert gives a hard upper bound, but the assume is a bit weaker on release builds, so if the code is used in a place where the upper bound is too small, the assume-version would continue to work on machines with enough memory and only crash "when needed" (aka when running oom).

💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2741214252)
`db442f3d6e...626cc06c69`: rebase due to conflicts

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2741231347)
`8c0ce1ca1c...dcb0be18ac`: rebase due to conflicts

💬 murchandamus commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2741233287)
The `long_term_fee_rate` and `fee_rate` are the same for all UTXOs in a single coin selection attempt, so if two UTXOs have the same fee, their inputs have the same weight, if they also have the same effective value, they are interchangeable. If they only have the same effective value but different fees, it means that they have differing weights. Similarly, waste simply scales with the weight across UTXOs, as it is weight times `long_term_fee_rate - fee_rate`. So, if two UTXOs have the same effe
...

🤔 Jassor909 reviewed a pull request: "refactor: Simplifies ProcessMessage for NetMsgType::TX"
(https://github.com/bitcoin/bitcoin/pull/32104#pullrequestreview-2703636828)
@DrahtBot