Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564038129)
Moved the change over, added you as a co-author - thanks
brunoerg closed a pull request: "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`"
(https://github.com/bitcoin/bitcoin/pull/31577)
💬 daniel1302 commented on pull request "Fix memory issue during chainstate initialization on 2GB box":
(https://github.com/bitcoin/bitcoin/pull/31575#discussion_r1898758548)
This function is declared but it looks unused in entire codebase.
💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776033)
Good idea, done in #27432 (referring now to the module docstring, which is also visible as `--help` output of the script).
💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776061)
Added a fixed timeout of 10 seconds, which should be more than enough given the tiny regtest UTXO set. I felt that it's not worth it to introduce a constant for that, but happy to add if others feel strongly.
💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776099)
Great idea, done in #27432.
💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1898776469)
The idea is to skip a test rather than fail, if the `sqlite3` module is not available (as far as I'm aware, the only supported distro where this could happen currently is FreeBSD). However, this indeed didn't work as expected since I was using `skip_if_no_sqlite` rather than `skip_if_no_py_sqlite3` (see #26882). Fixed in #27432.
💬 theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2564143915)
@tdb3 @romanz: Thanks for your reviews, much appreciated! Note that the first two commits which introduce the utxo-to-sqlite.py tool (+test) are part of the base PR #27432, so further comments on those changes would better fit there in the future. I took all of your suggestions and updated #27432 and rebased this PR on top of that again accordingly.
SecondPort closed a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31575)
💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898892756)
Should probably replace `// Support up to x-of-3 multisig txns as standard` with
`// Support up to x-of-MAX_BARE_MULTISIG_PUBKEYS_NUM multisig txns as standard`
💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898893779)
https://github.com/bitcoin/bitcoin/pull/31404/commits/088bbb81e34dfbe56fdeafaba2ddf756017e86e1: `MAX_BARE_MULTISIG_PUBKEYS_NUM` is an unsigned int so you can use `%u` format specifier instead of `%d`
💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898901503)
https://github.com/bitcoin/bitcoin/pull/31404/commits/50ebd73aeeb853328d1f4eadba77f2ca80a27a8c: AFAICT At this point, the `addmultisigaddress` operation is already complete (the scripts have been imported into the wallet and the destination is in the address book), throwing an error here may mislead the user into thinking the operation failed.

If you want the operation to fail if an invalid descriptor is inferred then this code needs to be moved up before the scripts are imported into the wal
...
💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898907274)
https://github.com/bitcoin/bitcoin/pull/31404/commits/50ebd73aeeb853328d1f4eadba77f2ca80a27a8c: The followig callers of ScriptToUniv need to be updated.

```diff
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 4cbe4a6062..5fcb9ff0d2 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1105,7 +1105,7 @@ static RPCHelpMan gettxout()
{RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT},
{RPC
...
📝 Jorah-M opened a pull request: "fix: typos fix in README.md"
(https://github.com/bitcoin/bitcoin/pull/31578)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
fanquake closed a pull request: "fix: typos fix in README.md"
(https://github.com/bitcoin/bitcoin/pull/31578)
💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898916436)
Sure, good catch. Still, this shouldn't happen because we are creating the destination internally. It is purely a sanity check.
💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898916762)
sure. Applied.
💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1898917082)
Sure. Done as suggested. Thanks.
👍 TheCharlatan approved a pull request: "validation: Send correct notification during snapshot completion"
(https://github.com/bitcoin/bitcoin/pull/31556#pullrequestreview-2524724192)
lgtm ACK bc43ecaf6dc0830a27296d3a29428814fed07bb1