💬 ryanofsky commented on pull request "WIP: scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#issuecomment-2472025885)
Updated 416860fc360b3d5aa1a0782ab8f8454b46e6d657 -> 47e30b40eca0eabf85d45e91fc247a74f3a346e8 ([`pr/scripty.1`](https://github.com/ryanofsky/bitcoin/commits/pr/scripty.1) -> [`pr/scripty.2`](https://github.com/ryanofsky/bitcoin/commits/pr/scripty.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/scripty.1..pr/scripty.2)) getting the remaining binaries (not just test_bitcoin and bitcoind) to build, simplifying the way optional and default values are used, making many other cleanups and
...
(https://github.com/bitcoin/bitcoin/pull/31260#issuecomment-2472025885)
Updated 416860fc360b3d5aa1a0782ab8f8454b46e6d657 -> 47e30b40eca0eabf85d45e91fc247a74f3a346e8 ([`pr/scripty.1`](https://github.com/ryanofsky/bitcoin/commits/pr/scripty.1) -> [`pr/scripty.2`](https://github.com/ryanofsky/bitcoin/commits/pr/scripty.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/scripty.1..pr/scripty.2)) getting the remaining binaries (not just test_bitcoin and bitcoind) to build, simplifying the way optional and default values are used, making many other cleanups and
...
💬 fjahr commented on pull request "wallet, rpc: Settxfeerate":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1839083891)
Looking back it seems like we haven't hidden them, just deprecated. That's also what I would suggest here.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1839083891)
Looking back it seems like we haven't hidden them, just deprecated. That's also what I would suggest here.
💬 adamandrews1 commented on pull request "addrman: cap the `max_pct` to not exceed the maximum number of addresses":
(https://github.com/bitcoin/bitcoin/pull/31235#issuecomment-2472050919)
Code Review ACK https://github.com/bitcoin/bitcoin/commit/9c5775c331e02dab06c78ecbb58488542d16dda7
(https://github.com/bitcoin/bitcoin/pull/31235#issuecomment-2472050919)
Code Review ACK https://github.com/bitcoin/bitcoin/commit/9c5775c331e02dab06c78ecbb58488542d16dda7
💬 polespinasa commented on pull request "wallet, rpc: Settxfeerate":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1839107296)
Do you have an example of deprecated RPC so I can compare on how it was done before?
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1839107296)
Do you have an example of deprecated RPC so I can compare on how it was done before?
🚀 glozow merged a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239)
(https://github.com/bitcoin/bitcoin/pull/30239)
💬 glozow commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2472070642)
Time to sweep the ephemeral top 9 commits
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2472070642)
Time to sweep the ephemeral top 9 commits
💬 fjahr commented on pull request "wallet, rpc: Settxfeerate":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1839141717)
Checking a few of these should give you an overview: https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+Deprecate+in%3Atitle+rpc+in%3Atitle+is%3Aclosed+
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1839141717)
Checking a few of these should give you an overview: https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+Deprecate+in%3Atitle+rpc+in%3Atitle+is%3Aclosed+
💬 tdb3 commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1839154278)
Was just focused on handling edge cases (and preventing harder to notice issues later). Not sure if there's a scenario involving orphaning that might play out (haven't thought through all scenarios).
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1839154278)
Was just focused on handling edge cases (and preventing harder to notice issues later). Not sure if there's a scenario involving orphaning that might play out (haven't thought through all scenarios).
👍 tdb3 approved a pull request: "doc: mention `descriptorprocesspsbt` in psbt.md"
(https://github.com/bitcoin/bitcoin/pull/31277#pullrequestreview-2431387717)
ACK ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e
Good catch
(https://github.com/bitcoin/bitcoin/pull/31277#pullrequestreview-2431387717)
ACK ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e
Good catch
💬 sipa commented on pull request "doc: mention `descriptorprocesspsbt` in psbt.md":
(https://github.com/bitcoin/bitcoin/pull/31277#issuecomment-2472112979)
ACK ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e
(https://github.com/bitcoin/bitcoin/pull/31277#issuecomment-2472112979)
ACK ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839276726)
Done. Also added a test for non-existent wallets and this uncovered a bug.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839276726)
Done. Also added a test for non-existent wallets and this uncovered a bug.
💬 achow101 commented on pull request "wallet: remove BDB dependency from wallet migration benchmark":
(https://github.com/bitcoin/bitcoin/pull/31241#issuecomment-2472224245)
While this does remove the BDB requirement, it doesn't remove the reliance on legacy wallet specific code that will be removed. Would it be possible to change this to create a wallet with a `LegacyDataSPKM` without utilizing any of the legacy wallet functions?
(https://github.com/bitcoin/bitcoin/pull/31241#issuecomment-2472224245)
While this does remove the BDB requirement, it doesn't remove the reliance on legacy wallet specific code that will be removed. Would it be possible to change this to create a wallet with a `LegacyDataSPKM` without utilizing any of the legacy wallet functions?
💬 kevkevinpal commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2472249506)
yes @rkrux you are right if I want to use a scripted diff then all the changes must be done using the script
I can remove the scripted diff and just squash them into a single diff since right now the script is getting very large already if that is preferred by others
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2472249506)
yes @rkrux you are right if I want to use a scripted diff then all the changes must be done using the script
I can remove the scripted diff and just squash them into a single diff since right now the script is getting very large already if that is preferred by others
💬 kevkevinpal commented on pull request "doc: mention `descriptorprocesspsbt` in psbt.md":
(https://github.com/bitcoin/bitcoin/pull/31277#issuecomment-2472268193)
ACK [ebb6cd8](https://github.com/bitcoin/bitcoin/pull/31277/commits/ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e)
(https://github.com/bitcoin/bitcoin/pull/31277#issuecomment-2472268193)
ACK [ebb6cd8](https://github.com/bitcoin/bitcoin/pull/31277/commits/ebb6cd82baf8406454b18afcb8ccb4e1dde0d43e)
💬 kevkevinpal commented on pull request "rpc: print P2WSH witScript in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1839327299)
I think it might make more sense to name it "witnessScript" instead of "witScript", It might be confusing to some if it is abbreviated
```suggestion
in.pushKV("witnessScript", std::move(witScript));
```
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1839327299)
I think it might make more sense to name it "witnessScript" instead of "witScript", It might be confusing to some if it is abbreviated
```suggestion
in.pushKV("witnessScript", std::move(witScript));
```
💬 kevkevinpal commented on pull request "rpc: print P2WSH witScript in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#issuecomment-2472281238)
can we squash 66ea8941de3dfd498319c6390655a165f6a5a92b and 84fb299de72ca9e08fe5029380a779d57d302347
to a single commit
(https://github.com/bitcoin/bitcoin/pull/31252#issuecomment-2472281238)
can we squash 66ea8941de3dfd498319c6390655a165f6a5a92b and 84fb299de72ca9e08fe5029380a779d57d302347
to a single commit
💬 ajtowns commented on pull request "validation: Remove RECENT_CONSENSUS_CHANGE validation result":
(https://github.com/bitcoin/bitcoin/pull/31269#issuecomment-2472317962)
To be able to differentiate `RECENT_CONSENSUS_CHANGE` errors, you'd need to revalidate txs up to three times instead of two (did this fail standardness? because of consensus rules? because of recent consensus rules?), which is probably more hassle (it's marginally easier for an attacker to use up CPU cycles) than it's worth (non-consensus compatible nodes stay well-connected for longer so are less likely to follow low-work chains). It's probably better dealt with via things like block-only-peers
...
(https://github.com/bitcoin/bitcoin/pull/31269#issuecomment-2472317962)
To be able to differentiate `RECENT_CONSENSUS_CHANGE` errors, you'd need to revalidate txs up to three times instead of two (did this fail standardness? because of consensus rules? because of recent consensus rules?), which is probably more hassle (it's marginally easier for an attacker to use up CPU cycles) than it's worth (non-consensus compatible nodes stay well-connected for longer so are less likely to follow low-work chains). It's probably better dealt with via things like block-only-peers
...
💬 Talej commented on pull request "doc: corrected lockunspent rpc quoting":
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2472323358)
Corrected additional RPC examples for gettxspendingprevout, createrawtransaction, signrawtransactionwithkey & addmultisigaddress
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2472323358)
Corrected additional RPC examples for gettxspendingprevout, createrawtransaction, signrawtransactionwithkey & addmultisigaddress
💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1839601505)
So you'd rather not take `else if (package.size() == 1 || //` still? I think it's an improvement for code comprehension if we end up handling single-tx here.
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1839601505)
So you'd rather not take `else if (package.size() == 1 || //` still? I think it's an improvement for code comprehension if we end up handling single-tx here.
💬 maflcko commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-2472823441)
Steps to reproduce on a fresh Ubuntu 24.04:
```
export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && apt install build-essential cmake pkg-config python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev libdb++-dev clang llvm valgrind -y && cmake -B ./bld-cmake -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_BENCH=OFF -DWITH_ZMQ=OFF
...
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-2472823441)
Steps to reproduce on a fresh Ubuntu 24.04:
```
export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && apt install build-essential cmake pkg-config python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev libdb++-dev clang llvm valgrind -y && cmake -B ./bld-cmake -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_BENCH=OFF -DWITH_ZMQ=OFF
...