Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 maflcko commented on issue "build: `CPPFLAGS` usage in OSS-Fuzz":
(https://github.com/bitcoin/bitcoin/issues/29385#issuecomment-1926627045)
Same in `ci/test/00_setup_env_native_fuzz_with_msan.sh`?
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1926654400)
I rebased on top of #29346 and pushed the result to: https://github.com/Sjors/bitcoin/tree/2024/02/sv2-poll-ellswift

Once EllSwift lands on the SRI side (or if I make a revert commit for it) I'll update this PR.
👋 maflcko's pull request is ready for review: "refactor: Allow CScript construction from any std::input_iterator"
(https://github.com/bitcoin/bitcoin/pull/29369)
💬 maflcko commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1926712573)
Removed unused `value_type`, which was wrong anyway, since it was cv qualified when it shouldn't.
👍 maflcko approved a pull request: "assumeutxo: Get rid of faked nTx and nChainTx values"
(https://github.com/bitcoin/bitcoin/pull/29370#pullrequestreview-1862394105)
lgtm
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1477998490)
Any reason to change the sequence id? I think this isn't changed in `loadtxoutset` either?
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1477981888)
Any reason to touch this LOC?
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1477998941)
Same (sequence id)
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1477996772)
```suggestion
index->nChainTx = 0;
```

isn't this always true? Maybe you meant to type `-` instead of `<`?

In any case, the test seems to be passing either way, so this is "dead" code, similar to a code comment.
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1478018977)
Can this be further limited to only mark the snap_block if it has ASSUMED_VALID set? Otherwise, nodes which never used loadtxoutset will assume that some block is a "snapshot block", even though they never used that feature.
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1478006900)
Is removing the validity check intended?
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1478022794)
Any reason to remove this check?
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1926747949)
> so i took time to test a NTA pinning scenario: [ariard/bitcoin@84e12b8](https://github.com/ariard/bitcoin/commit/84e12b8) which is known since https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-June/018011.html

To summarize the "network topology aware" scenario, what happens with Alice and Bob's channel is:
1. Bob broadcasts his commitment transaction + cpfp from his anchor
2. This transaction propagates to everyone except Alice, somehow. [1]
3. Alice broadcasts her commitment
...
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1926771908)
Thanks Luke, I agree that repeated string concatenation was not a particularly neat for creating a single string where we could simply modify in place. I've updated this further from your version to use a lambda in bf9b11d8f6c0af9b2c76674481e5905a00240cf5.

Re Windows. Yes, fs::permissions is "supported", but IIUC without devolving into access control lists, it all boils down to whether the write bit is enabled. I don't see the sense in supporting either of these things here personally. The de
...
💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1926795738)
ACK [fa9ef95](https://github.com/bitcoin/bitcoin/pull/29369/commits/fa9ef95b4c1114e2d8f7c3307fdf01446efee6fd)
💬 maflcko commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1926850992)
I guess this can be fixed by using std::string_view::starts_with
💬 maflcko commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1926859778)
I've created https://github.com/bitcoin/bitcoin/issues/26972 to track ideas on how to detect those silent build issues, or at least make them easier to spot.
💬 petertodd commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1926860376)
@glozow

> To summarize the "network topology aware" scenario, what happens with Alice and Bob's channel is:
>
> 1. Bob broadcasts his commitment transaction + cpfp from his anchor
>
> 2. This transaction propagates to everyone except Alice, somehow. [1]
>
> 3. Alice broadcasts her commitment transaction + cpfp from her anchor
>
> 4. Alice's transactions don't propagate because her commitment tx conflicts with Bob's commitment tx.
>
>
> This (1) requires the at
...
💬 maflcko commented on pull request "test: Assumeutxo with more than just coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/29354#issuecomment-1926882169)
rfm?