📝 m3dwards opened a pull request: "docs: ci multi-arch requires qemu"
(https://github.com/bitcoin/bitcoin/pull/29456)
On a fresh Debian system qemu isn't installed and therefore the multi-architecture CI system doesn't run.
This documentation notes that qemu is required and how to install it.
(https://github.com/bitcoin/bitcoin/pull/29456)
On a fresh Debian system qemu isn't installed and therefore the multi-architecture CI system doesn't run.
This documentation notes that qemu is required and how to install it.
💬 maflcko commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1495633116)
> If you want to test a large number, then better check against TX_MAX_STANDARD_VERSION + 1?
This check already exists two lines below.
> Does this still look meaningful?
Yes, the goal is to check the edge cases (minimum and maximum possible number)
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1495633116)
> If you want to test a large number, then better check against TX_MAX_STANDARD_VERSION + 1?
This check already exists two lines below.
> Does this still look meaningful?
Yes, the goal is to check the edge cases (minimum and maximum possible number)
💬 sipa commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1495653173)
It'd look a bit more natural using `= 0xffffffff` or `= std::numeric_limits<uint32_t>::max()`.
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1495653173)
It'd look a bit more natural using `= 0xffffffff` or `= std::numeric_limits<uint32_t>::max()`.
💬 hebasto commented on issue "ci_native_asan: UndefinedBehaviorSanitizer: null-pointer-use qt/test/wallettests.cpp:424:25 in":
(https://github.com/bitcoin-core/gui/issues/796#issuecomment-1954008330)
@maflcko
Does https://github.com/bitcoin-core/gui/pull/797 fix this issue for you?
(https://github.com/bitcoin-core/gui/issues/796#issuecomment-1954008330)
@maflcko
Does https://github.com/bitcoin-core/gui/pull/797 fix this issue for you?
💬 maflcko commented on pull request "test: Recognize dialog object by name":
(https://github.com/bitcoin-core/gui/pull/797#issuecomment-1954016527)
lgtm, Thanks!
(https://github.com/bitcoin-core/gui/pull/797#issuecomment-1954016527)
lgtm, Thanks!
💬 dergoegge commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1954024203)
I would like to see this in v27, can we add it to the milestone?
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1954024203)
I would like to see this in v27, can we add it to the milestone?
💬 fanquake commented on pull request "refactor: bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1954026863)
Note there are at least two defines missing from the scriptied diff (`_TIME_BITS` & `__MINGW_USE_VC2005_COMPAT`), if using Autoconf 2.72 , however that should not make a difference here. Looks like `WORDS_BIGENDIAN` is also missing, but that should als be ok.
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1954026863)
Note there are at least two defines missing from the scriptied diff (`_TIME_BITS` & `__MINGW_USE_VC2005_COMPAT`), if using Autoconf 2.72 , however that should not make a difference here. Looks like `WORDS_BIGENDIAN` is also missing, but that should als be ok.
💬 sipa commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#discussion_r1495673285)
Indeed, reverted (and also stuck closer to the former code).
(https://github.com/bitcoin/bitcoin/pull/29192#discussion_r1495673285)
Indeed, reverted (and also stuck closer to the former code).
✅ hebasto closed an issue: "ci_native_asan: UndefinedBehaviorSanitizer: null-pointer-use qt/test/wallettests.cpp:424:25 in"
(https://github.com/bitcoin-core/gui/issues/796)
(https://github.com/bitcoin-core/gui/issues/796)
🚀 hebasto merged a pull request: "test: Recognize dialog object by name"
(https://github.com/bitcoin-core/gui/pull/797)
(https://github.com/bitcoin-core/gui/pull/797)
💬 sipa commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1954030462)
@luke-jr There were a few expected encoding value tests, but I've expanded them significantly now.
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1954030462)
@luke-jr There were a few expected encoding value tests, but I've expanded them significantly now.
💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1495685692)
Yeah, it means "fee fraction", though the name isn't perfect - it really represents any ratio where the numerator is an `int64_t` and the denominator is an `int32_t`, with a sort order that tie-breaks by putting larger denominators last.
I don't think Chunk is a good name, as we're using that term for a subset of transactions. A `FeeFrac` is related, but represents the aggregate feerate of a chunk (or really of any set of transactions, or even the difference between the fees/sizes of two sets
...
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1495685692)
Yeah, it means "fee fraction", though the name isn't perfect - it really represents any ratio where the numerator is an `int64_t` and the denominator is an `int32_t`, with a sort order that tie-breaks by putting larger denominators last.
I don't think Chunk is a good name, as we're using that term for a subset of transactions. A `FeeFrac` is related, but represents the aggregate feerate of a chunk (or really of any set of transactions, or even the difference between the fees/sizes of two sets
...
💬 maflcko commented on pull request "refactor: bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1954058149)
> Looks like `WORDS_BIGENDIAN` is also missing, but that should also be ok.
It is covered in #29408, in any case.
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1954058149)
> Looks like `WORDS_BIGENDIAN` is also missing, but that should also be ok.
It is covered in #29408, in any case.
📝 BrandonOdiwuor opened a pull request: "doc: Fix gen-manpages to check build options"
(https://github.com/bitcoin/bitcoin/pull/29457)
### (!!! Disclaimer: This is an initial draft PR to get feedback on the approach)
Fixes https://github.com/bitcoin/bitcoin/issues/17506
gen-manapages.py should check enabled build options when generating docs
(https://github.com/bitcoin/bitcoin/pull/29457)
### (!!! Disclaimer: This is an initial draft PR to get feedback on the approach)
Fixes https://github.com/bitcoin/bitcoin/issues/17506
gen-manapages.py should check enabled build options when generating docs
💬 maflcko commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1495735626)
```
$ FUZZ=coin_grinder_is_optimal UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz /tmp/b
wallet/test/fuzz/coinselection.cpp:161:89: runtime error: implicit conversion from type 'CAmount' (aka 'long') of value -12 (64-bit, signed) to type 'size_type' (aka 'unsigned long') changed the value to 18446744073709551604 (64-bit, unsigned)
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-c
...
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1495735626)
```
$ FUZZ=coin_grinder_is_optimal UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz /tmp/b
wallet/test/fuzz/coinselection.cpp:161:89: runtime error: implicit conversion from type 'CAmount' (aka 'long') of value -12 (64-bit, signed) to type 'size_type' (aka 'unsigned long') changed the value to 18446744073709551604 (64-bit, unsigned)
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-c
...
💬 maflcko commented on pull request "doc: Fix gen-manpages to check build options":
(https://github.com/bitcoin/bitcoin/pull/29457#discussion_r1495746878)
My understanding would be that the check returns an error to recommend to build with the component, when a component is missing, no?
(https://github.com/bitcoin/bitcoin/pull/29457#discussion_r1495746878)
My understanding would be that the check returns an error to recommend to build with the component, when a component is missing, no?
💬 fjahr commented on pull request "doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()":
(https://github.com/bitcoin/bitcoin/pull/29355#issuecomment-1954125456)
> @fjahr i think drahtbot added that label because the commit starts with “doc:”
Yes, my question is why the "doc" is there in the name
(https://github.com/bitcoin/bitcoin/pull/29355#issuecomment-1954125456)
> @fjahr i think drahtbot added that label because the commit starts with “doc:”
Yes, my question is why the "doc" is there in the name
👍 fanquake approved a pull request: "refactor: bitcoin-config.h includes cleanup"
(https://github.com/bitcoin/bitcoin/pull/29404#pullrequestreview-1890385608)
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc
(https://github.com/bitcoin/bitcoin/pull/29404#pullrequestreview-1890385608)
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc
🚀 fanquake merged a pull request: "refactor: bitcoin-config.h includes cleanup"
(https://github.com/bitcoin/bitcoin/pull/29404)
(https://github.com/bitcoin/bitcoin/pull/29404)
💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1495827183)
Haven't tried, but I expect this will fix it:
```patch
// Ensure that each UTXO has at least an effective value of 1 sat
+ if (max_output_groups + max_spendable >= MAX_MONEY + group_pos.size()) break;
const CAmount eff_value{fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(1, MAX_MONEY - max_spendable - max_output_groups + group_pos.size())};
```
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1495827183)
Haven't tried, but I expect this will fix it:
```patch
// Ensure that each UTXO has at least an effective value of 1 sat
+ if (max_output_groups + max_spendable >= MAX_MONEY + group_pos.size()) break;
const CAmount eff_value{fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(1, MAX_MONEY - max_spendable - max_output_groups + group_pos.size())};
```