💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1663126949)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1663126949)
Done.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1663127067)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1663127067)
Done.
🚀 achow101 merged a pull request: "test: add coverage for `node` field of `getaddednodeinfo` RPC"
(https://github.com/bitcoin/bitcoin/pull/30339)
(https://github.com/bitcoin/bitcoin/pull/30339)
🚀 achow101 merged a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340)
(https://github.com/bitcoin/bitcoin/pull/30340)
💬 achow101 commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#issuecomment-2204366163)
ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6
(https://github.com/bitcoin/bitcoin/pull/30324#issuecomment-2204366163)
ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6
🚀 achow101 merged a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324)
(https://github.com/bitcoin/bitcoin/pull/30324)
💬 achow101 commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#issuecomment-2204380713)
ACK 7d55796c530f891493302059c6200d4a606c1ca9
(https://github.com/bitcoin/bitcoin/pull/30365#issuecomment-2204380713)
ACK 7d55796c530f891493302059c6200d4a606c1ca9
💬 achow101 commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#issuecomment-2204386784)
ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28
(https://github.com/bitcoin/bitcoin/pull/30267#issuecomment-2204386784)
ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28
🚀 achow101 merged a pull request: "#27307 follow-up: update mempool conflict tests + docs"
(https://github.com/bitcoin/bitcoin/pull/30365)
(https://github.com/bitcoin/bitcoin/pull/30365)
🚀 achow101 merged a pull request: "assumeutxo: Check snapshot base block is not in invalid chain"
(https://github.com/bitcoin/bitcoin/pull/30267)
(https://github.com/bitcoin/bitcoin/pull/30267)
👍 ryanofsky approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2154910604)
Code review ACK c1d6e525131cb9e54bbedb79ea64e2469a77aed8. Nice that this PR is +202 −261 lines and cleans a number of things up in addition to removing the globals.
I noticed https://github.com/bitcoin/bitcoin/issues/10754 while reviewing this, which suggests the idea of using a shared cache and could be something to follow up on. It also provides more background information about the caches.
re: https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2204306273
> This was a regressi
...
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2154910604)
Code review ACK c1d6e525131cb9e54bbedb79ea64e2469a77aed8. Nice that this PR is +202 −261 lines and cleans a number of things up in addition to removing the globals.
I noticed https://github.com/bitcoin/bitcoin/issues/10754 while reviewing this, which suggests the idea of using a shared cache and could be something to follow up on. It also provides more background information about the caches.
re: https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2204306273
> This was a regressi
...
💬 achow101 commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2204494467)
ACK a6821a0a4e1cf2aba679962289f4beeefd3d539a
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2204494467)
ACK a6821a0a4e1cf2aba679962289f4beeefd3d539a
💬 achow101 commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2204502009)
ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2204502009)
ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746
💬 achow101 commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2204502692)
ACK 2342b46c451658a418f8e28e50b2ad0e5abd284d
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2204502692)
ACK 2342b46c451658a418f8e28e50b2ad0e5abd284d
🚀 achow101 merged a pull request: "doc: use TRUC instead of v3 and add release note"
(https://github.com/bitcoin/bitcoin/pull/30272)
(https://github.com/bitcoin/bitcoin/pull/30272)
✅ achow101 closed an issue: "assumeutxo: nTx and nChainTx in RPC results are off"
(https://github.com/bitcoin/bitcoin/issues/29328)
(https://github.com/bitcoin/bitcoin/issues/29328)
🚀 achow101 merged a pull request: "rpc: Avoid getchaintxstats invalid results"
(https://github.com/bitcoin/bitcoin/pull/29720)
(https://github.com/bitcoin/bitcoin/pull/29720)
💬 Roasbeef commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1663344523)
> It's not clear to me why that is a problem though.
It gives those involved in the creation of the new parameters months of mining before the "public", which will skew the distribution of new testnet coins. If those that mined the chain for months before public release aren't also running faucets to easily disseminate the coins, then we end up in the same position where testnet coins are hard to obtain by developers.
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1663344523)
> It's not clear to me why that is a problem though.
It gives those involved in the creation of the new parameters months of mining before the "public", which will skew the distribution of new testnet coins. If those that mined the chain for months before public release aren't also running faucets to easily disseminate the coins, then we end up in the same position where testnet coins are hard to obtain by developers.
🤔 tdb3 reviewed a pull request: "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2155193780)
Approach ACK
Good work increasing clarity. In the past I've had a head-scratching moment with some of these before I learned the nuances, so this should help people.
Left a comment about release notes.
Tested:
- Ran unit/functional tests locally (passed)
- Manually exercised the following scenario (regtest):
- Created a transaction (and broadcasted).
- Generated one block to confirm.
- Attempted to call `sendrawtransaction` with the newly confirmed transaction.
- Receiv
...
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2155193780)
Approach ACK
Good work increasing clarity. In the past I've had a head-scratching moment with some of these before I learned the nuances, so this should help people.
Left a comment about release notes.
Tested:
- Ran unit/functional tests locally (passed)
- Manually exercised the following scenario (regtest):
- Created a transaction (and broadcasted).
- Generated one block to confirm.
- Attempted to call `sendrawtransaction` with the newly confirmed transaction.
- Receiv
...
💬 tdb3 commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1663380344)
Although it's only the error message (and the number stays the same), I still think its appropriate add a release note for the change to the user.
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes
> Release notes should be written for any PR that:
> ...
> makes any other visible change to the end-user experience.
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1663380344)
Although it's only the error message (and the number stays the same), I still think its appropriate add a release note for the change to the user.
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes
> Release notes should be written for any PR that:
> ...
> makes any other visible change to the end-user experience.