Bitcoin Core Github
44 subscribers
121K links
Download Telegram
βœ… willcl-ark closed an issue: "`libbitcoinconsensus.a` is unusable"
(https://github.com/bitcoin/bitcoin/issues/28779)
πŸ’¬ willcl-ark commented on issue "`libbitcoinconsensus.a` is unusable":
(https://github.com/bitcoin/bitcoin/issues/28779#issuecomment-1926512812)
Closing following #29189

It seems like the rust-bitcoinconsensus folks are sufficiently aware of the deprecation of this lib and migration to libbitcoinkernel, and plan to possibly start a [new crate](https://github.com/rust-bitcoin/rust-bitcoinconsensus/issues/66#issuecomment-1925928670) based on the kernel libs.
πŸ’¬ petertodd commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1926573232)
> I’m interested if there is a full-branch with package-relay + v3 tx policy available somewhere.

To be clear, package relay with package RBF is meant to be resistant to these kinds of network topology aware (NTA) pinning scenarios. With correctly implemented package relay it doesn't matter that a CPFP tx did a CPFP on top of a commitment transaction that no-one but the originator has: provided the fee (or with RBFR, fee-rate) outbids the competing commitment transaction, it will be replaced
...
πŸ’¬ fanquake commented on issue "When I make a highly concurrent request to bitcoin core, a new block appears and all my requests get blocked":
(https://github.com/bitcoin/bitcoin/issues/29384#issuecomment-1926579486)
> Package manager

Which package manager?

Can you provide any debug.log output?
πŸ’¬ maflcko commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-1926582804)
> > This class could `Assume` that the file was flushed/closed before the destructor is called?
>
> `Assume` is more for code correctness, not for external errors (like IO error).

The assumption would be that the close was called *before* the destructor is called. This is a code correctness question and seems like the perfect fit for Assume. That is, if the Assume fails, the code was missing a close, and the code must be changed to fix the internal bug. The Assume does not in any way check
...
πŸ’¬ ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1477927227)
> compare it with the resulting mining score instead of the new transaction’s individual feerate?

πŸ€” Not sure how to implement the change you suggested honestly.
The current behavour is checking if target fee rate of user wants to bump the transaction to is greater than `maxfeerate` which I believe is the new mining score of the bumped transaction no?
I might be wrong on my assumption though, would you clarify your comment please? thanks.
⚠️ hebasto opened an issue: "build: `CPPFLAGS` usage in OSS-Fuzz"
(https://github.com/bitcoin/bitcoin/issues/29385)
In OSS-Fuzz, dependencies are built with `DEBUG=1`, which implies usage of the additional preprocessor definitions:

```
$ make -C depends print-host_debug_CPPFLAGS HOST=x86_64-pc-linux-gnu DEBUG=1
...
host_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_ENABLE_DEBUG_MODE=1
```

However, explicit setting `CPPFLAGS` breaks this assumption:
```
$ make -C depends print-host_debug_CPPFLAGS HOST=x86_64-pc-linux-gnu DEBUG=1 CPPFLAGS="-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE"
...
πŸ’¬ 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?