Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 achow101 commented on issue "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" ":
(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.
💬 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
...
💬 TheCharlatan commented on issue "Libbitcoinkernel Project Tracking":
(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
...
📝 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.
👍 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
💬 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
💬 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
💬 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
...
💬 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
📝 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.
💬 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.
🚀 fanquake merged a pull request: "Fix typos in `subprocess.hpp`"
(https://github.com/bitcoin/bitcoin/pull/29849)
💬 theuni commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2049806807)
Want to upstream the crc32 patch to match the others we have sitting there?
💬 hebasto commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2049815951)
Concept ACK.
💬 laanwj commented on issue "util-{util,wallet}: undefined reference to `evhttp_uridecode'":
(https://github.com/bitcoin/bitcoin/issues/29654#issuecomment-2049819462)
`bitcoin-util` doesn't seem to be linked against libevent. It also doesn't actually use that function. Normally, the linker would drop unused object files, but somehow it doesn't in this user's case. It probably shouldn't be linking in that object in the first place.
💬 darosior commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#issuecomment-2049825002)
Reminder it's probably worth backporting #29853 to avoid hitting it in the fuzzer.
📝 darosior opened a pull request: "26.x: backport #29853 ("sign: don't assume we are parsing a sane Miniscript")"
(https://github.com/bitcoin/bitcoin/pull/29854)
Backports #29853.
📝 achow101 opened a pull request: "psbt: Check non witness utxo outpoint early"
(https://github.com/bitcoin/bitcoin/pull/29855)
A common issue that our fuzzers keep finding is that outpoints don't exist in the non witness utxos. Instead of trying to track this down and checking in various individual places, do the check early during deserialization. This also unifies the error message returned for this class of problems.