π¬ ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1477828459)
Yes its 10'000 sat/vb, I agree its high indeed Should I reduce it to maybe 1000 s/vb instead?
---
This is configurable option can be reduced by users to threshold they want.
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1477828459)
Yes its 10'000 sat/vb, I agree its high indeed Should I reduce it to maybe 1000 s/vb instead?
---
This is configurable option can be reduced by users to threshold they want.
π¬ glozow 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-1926494531)
Is it blocked forever or until the block is finished processing? Can you provide more details about what your highly concurrent request is?
(https://github.com/bitcoin/bitcoin/issues/29384#issuecomment-1926494531)
Is it blocked forever or until the block is finished processing? Can you provide more details about what your highly concurrent request is?
β
willcl-ark closed an issue: "`libbitcoinconsensus.a` is unusable"
(https://github.com/bitcoin/bitcoin/issues/28779)
(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.
(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
...
(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?
(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
...
(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.
(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"
...
(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-1926623041)
Reference: https://github.com/google/oss-fuzz/blob/c801e7aab5822e0c305fdb88dbe21b834d45afea/projects/bitcoin-core/build.sh#L38
(https://github.com/bitcoin/bitcoin/issues/29385#issuecomment-1926623041)
Reference: https://github.com/google/oss-fuzz/blob/c801e7aab5822e0c305fdb88dbe21b834d45afea/projects/bitcoin-core/build.sh#L38
π¬ 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`?
(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.
(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)
(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.
(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
(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?
(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?
(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)
(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.
(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.
(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.