💬 vasild commented on pull request "test: add test for malleated transaction with valid witness":
(https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2074738962)
Now I see! Multisig is another way to create "same_txid, different_wtxid" even though it does not fiddle with the `s` value. And so, both transactions are relayable. Cool. Best would be to have a function that produces those two transactions. That would need the private keys. I guess better be separate from `create_malleated_version()`.
(https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2074738962)
Now I see! Multisig is another way to create "same_txid, different_wtxid" even though it does not fiddle with the `s` value. And so, both transactions are relayable. Cool. Best would be to have a function that produces those two transactions. That would need the private keys. I guess better be separate from `create_malleated_version()`.
🤔 stratospher reviewed a pull request: "scripted-diff: adapt script error constant names in feature_taproot.py"
(https://github.com/bitcoin/bitcoin/pull/32415#pullrequestreview-2816937496)
ACK b5f580c. liked the consistency in script error names.
(https://github.com/bitcoin/bitcoin/pull/32415#pullrequestreview-2816937496)
ACK b5f580c. liked the consistency in script error names.
💬 maflcko commented on pull request "[WIP] rpc: add `clearmempool` command for regtest mode":
(https://github.com/bitcoin/bitcoin/pull/32418#issuecomment-2853377552)
> > What about invalidating the block, recording all mempool txs via `getrawmempool`, stopping the node, deleting `mempool.dat` and restarting?
>
> The wallets will reinsert the transactions unless you prevent them from doing so, but then you don't know what transactions to remove from the wallet. Restarting the node mid-test is also slow and annoying.
I think the idea was to "record"/remember the mempool txids to be abandoned later.
A third alternative could be to just backup the walle
...
(https://github.com/bitcoin/bitcoin/pull/32418#issuecomment-2853377552)
> > What about invalidating the block, recording all mempool txs via `getrawmempool`, stopping the node, deleting `mempool.dat` and restarting?
>
> The wallets will reinsert the transactions unless you prevent them from doing so, but then you don't know what transactions to remove from the wallet. Restarting the node mid-test is also slow and annoying.
I think the idea was to "record"/remember the mempool txids to be abandoned later.
A third alternative could be to just backup the walle
...
💬 maflcko commented on pull request "spam: trick Drahtbot into rendering a scammy link":
(https://github.com/bitcoin/bitcoin/pull/32422#issuecomment-2853433463)
Yeah, I think this probably fixed earlier as a side-effect of commit https://github.com/maflcko/DrahtBot/commit/b1871ab63bf6e2a1f8e8de7fc4c39bd56a52fe9d already. Of course it will never be 100% accurate, but it being able to find typos such as https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2065717959 seems minimally positive to me. Though, I can also drop the feature again, no strong opinion.
(https://github.com/bitcoin/bitcoin/pull/32422#issuecomment-2853433463)
Yeah, I think this probably fixed earlier as a side-effect of commit https://github.com/maflcko/DrahtBot/commit/b1871ab63bf6e2a1f8e8de7fc4c39bd56a52fe9d already. Of course it will never be 100% accurate, but it being able to find typos such as https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2065717959 seems minimally positive to me. Though, I can also drop the feature again, no strong opinion.
💬 aviv57 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2853437459)
Concept ack, I believe in the long run, we will benefit from less damage to the UTXO set by ordinals, inscription, etc.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2853437459)
Concept ack, I believe in the long run, we will benefit from less damage to the UTXO set by ordinals, inscription, etc.
💬 Eunovo commented on pull request "[EXPERIMENTAL] Schnorr batch verification for blocks":
(https://github.com/bitcoin/bitcoin/pull/29491#issuecomment-2853439571)
> Can you share the setup for these benchmarks so I can test them? Thanks!
OS is Ubuntu 24.04.2 LTS
Output from `lspcu`
```
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Address sizes: 48 bits physical, 48 bits virtual
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: AuthenticAMD
Model name: AMD Ryzen 9 8945HS w/ Radeon 780M Graphics
CPU family:
...
(https://github.com/bitcoin/bitcoin/pull/29491#issuecomment-2853439571)
> Can you share the setup for these benchmarks so I can test them? Thanks!
OS is Ubuntu 24.04.2 LTS
Output from `lspcu`
```
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Address sizes: 48 bits physical, 48 bits virtual
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: AuthenticAMD
Model name: AMD Ryzen 9 8945HS w/ Radeon 780M Graphics
CPU family:
...
💬 maflcko commented on pull request "descriptors: taproot partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2853470218)
Could turn into draft while CI is red?
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2853470218)
Could turn into draft while CI is red?
💬 laanwj commented on issue "rpc: actually deprecate `rpcuser` & `rpcpass`":
(https://github.com/bitcoin/bitcoin/issues/29240#issuecomment-2853477663)
> One thing with cookie auth, that currently needs hacks, is allowing cookie access for other users. https://github.com/bitcoin/bitcoin/pull/28167
Well this one was merged, we can nuke them now!
No, to be fair, i'm kind of divided on this issue too-i don't think it actually hurts to allow `rpcuser`/`rpcpasswd` to configure one account for easy single-user-single-application setups, yes it's less secure but this is all about local files and APIs, often if hackers get access to your server you'r
...
(https://github.com/bitcoin/bitcoin/issues/29240#issuecomment-2853477663)
> One thing with cookie auth, that currently needs hacks, is allowing cookie access for other users. https://github.com/bitcoin/bitcoin/pull/28167
Well this one was merged, we can nuke them now!
No, to be fair, i'm kind of divided on this issue too-i don't think it actually hurts to allow `rpcuser`/`rpcpasswd` to configure one account for easy single-user-single-application setups, yes it's less secure but this is all about local files and APIs, often if hackers get access to your server you'r
...
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2853490844)
> Could probably also remove the proxy functions we have in the test framework for those RPC commands. Look [at this line](https://github.com/bitcoin/bitcoin/blob/fc6346dbc8dc3db40aad4079210332b5f8b332ed/test/functional/test_framework/test_node.py#L979) for example (same for importpubkey, importprivkey and addmultisigaddress).
See https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2063194316
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2853490844)
> Could probably also remove the proxy functions we have in the test framework for those RPC commands. Look [at this line](https://github.com/bitcoin/bitcoin/blob/fc6346dbc8dc3db40aad4079210332b5f8b332ed/test/functional/test_framework/test_node.py#L979) for example (same for importpubkey, importprivkey and addmultisigaddress).
See https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2063194316
💬 maflcko commented on issue "Enable `importprivkey`, `addmultisigaddress` in descriptor wallets":
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2853501498)
For reference, BDB removal https://github.com/bitcoin/bitcoin/pull/31250 was merged (for 30.0)
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2853501498)
For reference, BDB removal https://github.com/bitcoin/bitcoin/pull/31250 was merged (for 30.0)
💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2074860730)
Doesn't the presence of the original cache-usage-update indicate that the comment isn't enough?
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2074860730)
Doesn't the presence of the original cache-usage-update indicate that the comment isn't enough?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074890106)
Same for `NotifyWatchonlyChanged`: It doesn't look to be called anywhere, so can be removed.
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074890106)
Same for `NotifyWatchonlyChanged`: It doesn't look to be called anywhere, so can be removed.
👍 maflcko approved a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2817225709)
re-review ACK 1c0d89358e 🎽
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-review ACK 1c0d89358e 🎽
PZKgfTbviYeE2sXUccefH
...
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2817225709)
re-review ACK 1c0d89358e 🎽
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-review ACK 1c0d89358e 🎽
PZKgfTbviYeE2sXUccefH
...
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074911165)
nit in b6bb744a881d465d15b40413a2753ca4865e851a: Seems fine to fully remove this, given that it isn't needed anyway after commit 1111aecbb58d6e37d430d477ac43f52811fd97d9
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074911165)
nit in b6bb744a881d465d15b40413a2753ca4865e851a: Seems fine to fully remove this, given that it isn't needed anyway after commit 1111aecbb58d6e37d430d477ac43f52811fd97d9
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074935902)
nit in a29101b56541d4bf72fcf69460e8eb2a8cc33979: I guess you wanted to leave some of those for follow-ups, but the `doc/descriptors.md` one seems in-scope?
```
$ git grep -i 'legacy wallet' 1c0d89358e12fc871e99c8304d5cb50965cf7b9d doc/descriptors.md doc/managing-wallets.md src/bitcoin-wallet.cpp src/wallet src/script/ test/
1c0d89358e12fc871e99c8304d5cb50965cf7b9d:doc/descriptors.md:- `importmulti` takes as input descriptors to import into a legacy wallet
1c0d89358e12fc871e99c8304d5cb509
...
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074935902)
nit in a29101b56541d4bf72fcf69460e8eb2a8cc33979: I guess you wanted to leave some of those for follow-ups, but the `doc/descriptors.md` one seems in-scope?
```
$ git grep -i 'legacy wallet' 1c0d89358e12fc871e99c8304d5cb50965cf7b9d doc/descriptors.md doc/managing-wallets.md src/bitcoin-wallet.cpp src/wallet src/script/ test/
1c0d89358e12fc871e99c8304d5cb50965cf7b9d:doc/descriptors.md:- `importmulti` takes as input descriptors to import into a legacy wallet
1c0d89358e12fc871e99c8304d5cb509
...
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074950668)
(same commit): remove
```
test/sanitizer_suppressions/tsan:race:BerkeleyBatch
```
?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074950668)
(same commit): remove
```
test/sanitizer_suppressions/tsan:race:BerkeleyBatch
```
?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074924915)
(same commit): Forgot to remove `deprecatedrpc=create_bdb`?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074924915)
(same commit): Forgot to remove `deprecatedrpc=create_bdb`?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074946960)
Yeah, I meant that flushing isn't used by any chain client and I don't think there is much need to keep the dead code, but can be done in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074946960)
Yeah, I meant that flushing isn't used by any chain client and I don't think there is much need to keep the dead code, but can be done in a follow-up.
💬 metasmile commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2853693834)
The main arguments supporting the removal of the OP_RETURN size limit appear to be as follows:
- If a transaction includes a large amount of data, it will naturally incur significant fees, which serve as a de facto limitation.
However, it must be seriously considered that the block space can still be abused by certain entities or groups who are indifferent to high fees.
- It has been argued that due to the currently open parameters, OP_RETURN has already been used in a way that is effecti
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2853693834)
The main arguments supporting the removal of the OP_RETURN size limit appear to be as follows:
- If a transaction includes a large amount of data, it will naturally incur significant fees, which serve as a de facto limitation.
However, it must be seriously considered that the block space can still be abused by certain entities or groups who are indifferent to high fees.
- It has been argued that due to the currently open parameters, OP_RETURN has already been used in a way that is effecti
...
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074994436)
follow-up nit in 18a1bd87813326ac2b1df43b8d37e5815f1ec033: Could convert those to descriptor wallets?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2074994436)
follow-up nit in 18a1bd87813326ac2b1df43b8d37e5815f1ec033: Could convert those to descriptor wallets?