Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 jonatack opened a pull request: "Remove no-longer-needed NOLINTNEXTLINE since C++20 upgrade"
(https://github.com/bitcoin/bitcoin/pull/29202)
This was a temporary workaround for C++17 in https://github.com/bitcoin/bitcoin/pull/28583 and should no longer be needed for the CI tidy task now that the project has moved to C++20.
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880319951)
> why not just apply it to all peers

Do you mean why not apply it to automatic peers as well per default (and ignoring the information from the service flag)? I thought the idea was to save one disconnection / reconnection, and the time spent doing these when we probably know better. But as I wrote in https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1854197471, I think that we depend on the reconnection mechanism for automatic connection as well, because anyone can add a wrong "v2"
...
💬 kevkevinpal commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1880357066)
Concept ACK [2422b90](https://github.com/bitcoin/bitcoin/pull/29156/commits/2422b90978f4ea13ee49954598dc8aef841e36df)

I've done a little code review and so far it is looking good, I think it would be cool if instead of always using the pubkeys in the same order when decaying we could randomly use different signers, I just quickly tested it worked the other way around with this diff (not sure how you feel about adding this randomness)

```
for m in range(self.M):
-
...
jonatack closed a pull request: "refactor: use push_back in TaprootBuilder::Add(), rm NOLINTNEXTLINE and comment"
(https://github.com/bitcoin/bitcoin/pull/29202)
💬 totient commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1880361556)
> > It's meant to limit extra data in transactions. OP_RETURN was supposed to be the only tolerated way to do that.
>
> Can you add any references for this? because the GitHub and git history very clearly contradicts your statements ([#29187 (comment)](https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1878972398)).

"There was been some confusion and misunderstanding in the community, regarding the OP_RETURN feature in 0.9 and data in the blockchain. This change is not an endorse
...
🤔 BrandonOdiwuor reviewed a pull request: "refactor(tidy): Use C++20 contains method"
(https://github.com/bitcoin/bitcoin/pull/29191#pullrequestreview-1808113803)
Concept ACK using ::contains(Key& key) (C++20) to check for elements
💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1880584066)
> Pre-review note: have you considered that wallet_import_rescan.py is a legacy wallet only test?

Ah good point. I added a commit to run this test with descriptors as I couldn't find a reason why we don't do this. Lmk if this seems sufficient / ok to do?
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359148)
taken
👍 brunoerg approved a pull request: "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption"
(https://github.com/bitcoin/bitcoin/pull/29200#pullrequestreview-1808534184)
crACK 9d728916b27e18efc6f8839770ed5ec14789fc08
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359693)
I've added an `Assume` checking that there is a v3 tx in the package and exiting early if not
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359794)
taken
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359931)
taken
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360019)
fixed
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360107)
taken, thanks
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360587)
because otherwise it is always 0
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360834)
taken
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/29201)
💬 maflcko commented on pull request "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py":
(https://github.com/bitcoin/bitcoin/pull/29067#issuecomment-1880721233)
> over int.from_bytes(bs, "little")

In python3.11, one could also write `int.from_bytes(bs)` for single-byte conversions.
💬 glozow commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#issuecomment-1880722797)
Still working on this? I'll be available to re-review quickly after comments are addressed.