👍 tdb3 approved a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2178157576)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb
Tested with a similar approach to https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2151672225
Created an LXC container with an external and loopback IPv4 address, and with a loopback IPv6 address.
```
root@test30245:~/bitcoin# ip a | grep inet
inet 127.0.0.1/8 scope host lo
inet6 ::1/128 scope host noprefixroute
inet 192.168.10.81/24 brd 192.168.10.255 scope global eth0
```
Inserted some extra log statments
...
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2178157576)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb
Tested with a similar approach to https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2151672225
Created an LXC container with an external and loopback IPv4 address, and with a loopback IPv6 address.
```
root@test30245:~/bitcoin# ip a | grep inet
inet 127.0.0.1/8 scope host lo
inet6 ::1/128 scope host noprefixroute
inet 192.168.10.81/24 brd 192.168.10.255 scope global eth0
```
Inserted some extra log statments
...
🚀 fanquake merged a pull request: "test: [refactor] Pass TestOpts"
(https://github.com/bitcoin/bitcoin/pull/30407)
(https://github.com/bitcoin/bitcoin/pull/30407)
💬 fanquake commented on pull request "test: Add arguments for creating a slimmer TestingSetup":
(https://github.com/bitcoin/bitcoin/pull/30399#discussion_r1678096936)
#30407 is now merged.
(https://github.com/bitcoin/bitcoin/pull/30399#discussion_r1678096936)
#30407 is now merged.
👍 ryanofsky approved a pull request: "log: LogError with FlatFilePos in UndoReadFromDisk"
(https://github.com/bitcoin/bitcoin/pull/30428#pullrequestreview-2178200481)
Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent
(https://github.com/bitcoin/bitcoin/pull/30428#pullrequestreview-2178200481)
Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent
💬 theStack commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2228934930)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2228934930)
Concept ACK
👍 dergoegge approved a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2178208060)
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6
(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
(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.
(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
(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)
(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.
(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.
(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.
(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
(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
...
(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.
(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
...
(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
...
(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.
(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.
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2229281988)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/263.