🚀 glozow merged a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735)
(https://github.com/bitcoin/bitcoin/pull/29735)
⚠️ dergoegge opened an issue: "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" "
(https://github.com/bitcoin/bitcoin/issues/29851)
```
$ echo "ZmluYWxpemVwc2J0XABwc2J0/wEAo6ujYwMDAwMDmAAAANUABQAAY2xlYXJiDAAAAAAAAABgAPX///8A6AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACTAAAAAAAAAAAAAABgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAAAwoDAwMDAwMDAwMDAwAK8wEDAHFkAAAAAAAAAAAAZQAAAAAAAAAAANzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3NzcAAAAAD8/Pz8/Pz8/Pz8/Pz8/P0Y/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz89Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8
...
(https://github.com/bitcoin/bitcoin/issues/29851)
```
$ echo "ZmluYWxpemVwc2J0XABwc2J0/wEAo6ujYwMDAwMDmAAAANUABQAAY2xlYXJiDAAAAAAAAABgAPX///8A6AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACTAAAAAAAAAAAAAABgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAAAwoDAwMDAwMDAwMDAwAK8wEDAHFkAAAAAAAAAAAAZQAAAAAAAAAAANzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3NzcAAAAAD8/Pz8/Pz8/Pz8/Pz8/P0Y/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz89Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8
...
💬 maflcko commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1560977607)
```suggestion
rand_fee = satoshi_round(fee_increment * (1.1892 ** random.randint(0, 28)), rounding=ROUND_DOWN)
```
Why the decimal?
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1560977607)
```suggestion
rand_fee = satoshi_round(fee_increment * (1.1892 ** random.randint(0, 28)), rounding=ROUND_DOWN)
```
Why the decimal?
💬 maflcko commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1560978096)
```suggestion
def satoshi_round(amount, *, rounding) -> Decimal:
```
why the default, if the goal is to not use a default?
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1560978096)
```suggestion
def satoshi_round(amount, *, rounding) -> Decimal:
```
why the default, if the goal is to not use a default?
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1560983729)
added something similar
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1560983729)
added something similar
💬 achow101 commented on issue "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049639006)
The `CHECK_NONFATAL` is in `FromPKBytes`: https://github.com/bitcoin/bitcoin/blob/bdb33ec51986570ea17406c83bad2c955ae23186/src/script/sign.cpp#L298 which is called when getting the miniscript for a script.
This particular script is `173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292`
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049639006)
The `CHECK_NONFATAL` is in `FromPKBytes`: https://github.com/bitcoin/bitcoin/blob/bdb33ec51986570ea17406c83bad2c955ae23186/src/script/sign.cpp#L298 which is called when getting the miniscript for a script.
This particular script is `173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292`
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2049639073)
Rebased for #29735 and fixed failure.
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2049639073)
Rebased for #29735 and fixed failure.
💬 achow101 commented on issue "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049640339)
cc @darosior
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049640339)
cc @darosior
💬 maflcko commented on issue "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049649573)
I don't think this is a fuzz issue. The RPC is `finalizepsbt` and it should be possible to hit it in `finalizepsbt` RPC as well.
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049649573)
I don't think this is a fuzz issue. The RPC is `finalizepsbt` and it should be possible to hit it in `finalizepsbt` RPC as well.
💬 maflcko commented on issue "fuzz, rpc: Internal bug in `finalizepsbt` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049678227)
Somewhat smaller input for the RPC:
```
$ ./src/bitcoin-cli finalizepsbt cHNidP8BAKOro2MDAwMDA5gAAAbVAAUAAGNsZWFyYgwAAAAAAAAAYAD1////AOgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAkwAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAADwAAAAAAAAAAAAAAAAAAAAAAAAMKAwMDAwMDAwMDAwMACvMBAwBxZAAAAAAAAAAAAGUAAAAODgAAAAEBKwL/ZABlcAAAIlEgAwMcHBwDAwAL8wEDJnBmAQAAAAAAAAAAAAAAAAAAgACiFQILCwsAAAAAAAAAAP////8AAACODg4ODg4ODg4ODgAADg5wAAAiUSADAxwcHAMDAAvzAQMmcGYBAAAAAAAAAAAAAAAAAACAAAAAAAAAACSJiYmJyon6AAAAgAAAAAAAAAD
...
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049678227)
Somewhat smaller input for the RPC:
```
$ ./src/bitcoin-cli finalizepsbt cHNidP8BAKOro2MDAwMDA5gAAAbVAAUAAGNsZWFyYgwAAAAAAAAAYAD1////AOgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAkwAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAADwAAAAAAAAAAAAAAAAAAAAAAAAMKAwMDAwMDAwMDAwMACvMBAwBxZAAAAAAAAAAAAGUAAAAODgAAAAEBKwL/ZABlcAAAIlEgAwMcHBwDAwAL8wEDJnBmAQAAAAAAAAAAAAAAAAAAgACiFQILCwsAAAAAAAAAAP////8AAACODg4ODg4ODg4ODgAADg5wAAAiUSADAxwcHAMDAAvzAQMmcGYBAAAAAAAAAAAAAAAAAACAAAAAAAAAACSJiYmJyon6AAAAgAAAAAAAAAD
...
💬 TheCharlatan commented on issue "Libbitcoinkernel Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-2049678827)
Updated with initial description of stage 2.
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-2049678827)
Updated with initial description of stage 2.
💬 laanwj commented on issue "ci: Lower and unify default stack size":
(https://github.com/bitcoin/bitcoin/issues/29840#issuecomment-2049719550)
FWIW, the command on Linux to set the stack size for a session is `ulimit -s 512`. Where the amount is in KiB. This sets the maximum for the main thread, as well as additional theads created.
> Any reason to not use half of the minimum, to more likely catch edge cases that would happen on unknown operating systems
i'm not sure, yes in principle this makes sense, but also the work that goes in having our code run with say a 256KiB stack seems unnecessary work if the lowest we know used is 5
...
(https://github.com/bitcoin/bitcoin/issues/29840#issuecomment-2049719550)
FWIW, the command on Linux to set the stack size for a session is `ulimit -s 512`. Where the amount is in KiB. This sets the maximum for the main thread, as well as additional theads created.
> Any reason to not use half of the minimum, to more likely catch edge cases that would happen on unknown operating systems
i'm not sure, yes in principle this makes sense, but also the work that goes in having our code run with say a 256KiB stack seems unnecessary work if the lowest we know used is 5
...
📝 fanquake opened a pull request: "[WIP] build: remove need to test for endianness"
(https://github.com/bitcoin/bitcoin/pull/29852)
We can cherry-pick one commit from upstream leveldb, make the same change in crc32c, and then ultimately drop our build infra for testing endianness.
Not for merging until subtrees are updated etc. Would also mean less code that we need to port to CMake.
(https://github.com/bitcoin/bitcoin/pull/29852)
We can cherry-pick one commit from upstream leveldb, make the same change in crc32c, and then ultimately drop our build infra for testing endianness.
Not for merging until subtrees are updated etc. Would also mean less code that we need to port to CMake.
👍 BrandonOdiwuor approved a pull request: "test: remove duplicated ban test"
(https://github.com/bitcoin/bitcoin/pull/29688#pullrequestreview-1994237763)
ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf
Looks good to me
(https://github.com/bitcoin/bitcoin/pull/29688#pullrequestreview-1994237763)
ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf
Looks good to me
💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1561056518)
I removed it and this seems to run fine, at least in my system
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1561056518)
I removed it and this seems to run fine, at least in my system
💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1561059463)
Oh, my point was that it could have also been defined as `0s` there too, but I won't insist
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1561059463)
Oh, my point was that it could have also been defined as `0s` there too, but I won't insist
💬 darosior commented on issue "fuzz, rpc: Internal bug in `finalizepsbt` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049765453)
A repro as a unit test:
```cpp
BOOST_AUTO_TEST_CASE(sign_invalid_miniscript)
{
FillableSigningProvider keystore;
SignatureData sig_data;
CMutableTransaction prev, curr;
const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")};
TaprootBuilder builder;
builder.Add(0, {invalid_pubkey}, 0xc0);
XOnlyPubKey dummy{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a
...
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049765453)
A repro as a unit test:
```cpp
BOOST_AUTO_TEST_CASE(sign_invalid_miniscript)
{
FillableSigningProvider keystore;
SignatureData sig_data;
CMutableTransaction prev, curr;
const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")};
TaprootBuilder builder;
builder.Add(0, {invalid_pubkey}, 0xc0);
XOnlyPubKey dummy{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a
...
💬 laanwj commented on issue "v27.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/29697#issuecomment-2049782942)
> What's the status of the disabling of JoinMarket?
joinmarket (master) works with v27.0rc1, both sendpayment and the yield generator
(https://github.com/bitcoin/bitcoin/issues/29697#issuecomment-2049782942)
> What's the status of the disabling of JoinMarket?
joinmarket (master) works with v27.0rc1, both sendpayment and the yield generator
📝 darosior opened a pull request: "sign: don't assume we are parsing a sane TapMiniscript"
(https://github.com/bitcoin/bitcoin/pull/29853)
The script provided for signature might be externally provided, for instance by way of 'finalizepsbt'. Therefore the script might be ill-crafted, so don't assume pubkeys are always 32 bytes.
Thanks to Niklas for finding this.
FIxes https://github.com/bitcoin/bitcoin/issues/29851.
(https://github.com/bitcoin/bitcoin/pull/29853)
The script provided for signature might be externally provided, for instance by way of 'finalizepsbt'. Therefore the script might be ill-crafted, so don't assume pubkeys are always 32 bytes.
Thanks to Niklas for finding this.
FIxes https://github.com/bitcoin/bitcoin/issues/29851.
💬 theuni commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2049803970)
Concept ACK. Looking at the [leveldb godbolt link](https://godbolt.org/z/45S0ID), this is nicely optimized everywhere except MSVC. I'm ok with a possible regression there for the sake of the cleanup.
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2049803970)
Concept ACK. Looking at the [leveldb godbolt link](https://godbolt.org/z/45S0ID), this is nicely optimized everywhere except MSVC. I'm ok with a possible regression there for the sake of the cleanup.