Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 dergoegge approved a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2178208060)
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6
👍 dergoegge approved a pull request: "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305"
(https://github.com/bitcoin/bitcoin/pull/28263#pullrequestreview-2178213334)
tACK 8607773750e60931e51a33e48cd077a1dedf9db3
🤔 glozow reviewed a pull request: "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)"
(https://github.com/bitcoin/bitcoin/pull/28923#pullrequestreview-2178233778)
light review ACK fe92c15f0c44d1405b9048306736bd0eae868506

Not super familiar with this area, did some code review to try to make sure the `VerifyScript` in `ProduceSignature` and end of `SignTransaction` have the same arguments, and `complete` can't e.g. be from a `DummySignatureChecker`. AFAICT the `MutableTransactionSignatureCreator::Checker()` and `TransactionSignatureChecker(...)` here should be equivalent.
🤔 mzumsande reviewed a pull request: "test: Fix intermittent failure in p2p_v2_misbehaving.py"
(https://github.com/bitcoin/bitcoin/pull/30420#pullrequestreview-2178308012)
Code Review ACK c6d43367a1ec47c004991143f031417c4e4b8095
🚀 ryanofsky merged a pull request: "log: LogError with FlatFilePos in UndoReadFromDisk"
(https://github.com/bitcoin/bitcoin/pull/30428)
💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2229091476)
> @pablomartin4btc
>
> > ... was introduced in #718...
>
> Sure about PR number? ('cause there is no such a number in this repo)

oh! my bad.. a typo there... how lucky! I'll play the lottery with that one today... I meant #708, I think it's the only place related to the "mask value" where I switched to the overview tab when the mask value checkbox on the menu is ticked but still need to check it.
💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#issuecomment-2229124568)
@pinheadmz @vasild

Ready for you to take a second look if you have any time.
👍 hebasto approved a pull request: "gui: rendering an amp characters in the wallet name for QMenu"
(https://github.com/bitcoin-core/gui/pull/828#pullrequestreview-2178389009)
re-ACK 8233ee41ab9648cd0c3bd78bc2a8d692a54d9ea0.

CI failures are unrelated.
💬 hebasto commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2229220553)
Another one: https://cirrus-ci.com/task/5588622949220352
📝 furszy converted_to_draft a pull request: "refactor: interfaces, make 'createTransaction' less error-prone "
(https://github.com/bitcoin-core/gui/pull/807)
Bundle all function's outputs inside the `util::Result` returned object.

Removals:
- The input-output 'change_pos' ref arg from `createTransaction`, which has been a source of bugs in the past.
- The 'fee' ref arg from `createTransaction`, which is currently only set when the transaction creation process succeeds.
- The currently unreachable `AmountWithFeeExceedsBalance` error (more info about its re-introduction at https://github.com/bitcoin/bitcoin/pull/25269).

Additionally, this PR m
...
💬 furszy commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2229224539)
Drafted until https://github.com/bitcoin/bitcoin/pull/25269 gets merged. This is a continuation of that work.
💬 TheBlueMatt commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2229233817)
> This PR returns a BlockTemplate interface which causes the node to hold on to the template and its block.
> The goal is to be able to quickly send NewTemplate and after that to copy all transactions to the external application so it can respond to RequestTransactionData and SubmitSolution.
> I assume your concern is that this would make SubmitSolution too slow because the external application needs to pass the whole block back via IPC?

Ah, okay, that was my only real concern. Sending tran
...
👍 pinheadmz approved a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2178514340)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb

Reviewed updated code change and tested in linux VM with local ipv6 only.

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 23333b7ed243071c9b4e4f04c727556d8065acbb
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaVfcgACgkQ5+KYS2KJ
yTorBw/7BFoooqdMN+Zb000m/qKsUoMhW36vZBdGTuxt3xkWrRHlggm8ym/fQfen
jt9VcWYl6vmZFnU+nUvJbdN+GLTdf+fDT8vdJdYKjKSxmflEIGLRnf5Pktf2E
...
💬 pinheadmz commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1678310386)
If you need to retouch, you might consider using a local `addrinfo` variable for the second call since it serves a specific purpose only inside this if condition. It doesn't matter because it's not like it gets used again later, just something I noticed.
💬 hebasto commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2229281988)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/263.
💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1678338506)
Don't have strong feeling either way but there is an argument that doing it with one variable means any changes to flags passed to the first call get automatically added to the second call which is I think better default behaviour.
🤔 mzumsande reviewed a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2178557577)
Concept ACK - don't know the txrequest logic very well though.

The PR description should be updated (`UpdatedBlockTipSync` was renamed).
💬 mzumsande commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1678336506)
I think you meant `m_tx_download_mutex`
👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2178579385)
Light code review ACK 408c11e9bced08c51a7645a2de2c39c18e4360d9. This looks good, but I didn't review the getCoinbaseMerklePath implementation closely to check that it is computing the right thing.
💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1678349206)
In commit "Have createNewBlock return BlockTemplate interface" (4af3d9a483e4107301f0f0a42da781dfa79acb82)

Should be possible to avoid getBlockHeader calls by just using the `block` variable:

```diff
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -941,7 +941,7 @@ static RPCHelpMan getblocktemplate()
result.pushKV("vbavailable", std::move(vbavailable));
result.pushKV("vbrequired", int(0));

- result.pushKV("previousblockhash", block_template->getBlockHeader().hashP
...