💬 tnndbtc commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2107732347)
@ismaelsadeeq Thanks for the review. To achieve what you've suggested, I am thinking of following proposal:
1) by default, set a good randomseed (example, 298652989016148828) that will ensure the feerate is always returned. This will test the happy path.
2) run this test with another option, which set the problematic randomseed (example, 2986529890161488286) that will cause feerate fail to return. When this happens, validate that the randomseed is indeed the designated one.
This proposal
...
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2107732347)
@ismaelsadeeq Thanks for the review. To achieve what you've suggested, I am thinking of following proposal:
1) by default, set a good randomseed (example, 298652989016148828) that will ensure the feerate is always returned. This will test the happy path.
2) run this test with another option, which set the problematic randomseed (example, 2986529890161488286) that will cause feerate fail to return. When this happens, validate that the randomseed is indeed the designated one.
This proposal
...
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107732522)
Since this RPC allowed all other types to be aliases for watchonly stuff, I added the backwards compatibility guard for any scripts/programs that may be setting the old parameter instead of an options object. The parameter is ignored because watchonly is no longer a thing, but this way it doesn't throw for any callers written in that way.
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107732522)
Since this RPC allowed all other types to be aliases for watchonly stuff, I added the backwards compatibility guard for any scripts/programs that may be setting the old parameter instead of an options object. The parameter is ignored because watchonly is no longer a thing, but this way it doesn't throw for any callers written in that way.
💬 TheCharlatan commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2107740514)
I think it would be ok, but we'd have to change the binary format here to skip over the empty `0` entry for the coinbase?
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2107740514)
I think it would be ok, but we'd have to change the binary format here to skip over the empty `0` entry for the coinbase?
🤔 mzumsande reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2869017740)
Code Review ACK 35bcd8eed38130445aef5ebe217ab42248fa6f18
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2869017740)
Code Review ACK 35bcd8eed38130445aef5ebe217ab42248fa6f18
💬 jonatack commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2910454957)
Somehow I hadn't seen this PR yet.
If I understand correctly, this change is only for if a node is configured with `-i2pacceptincoming=0` to not accept inbound I2P connections?
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2910454957)
Somehow I hadn't seen this PR yet.
If I understand correctly, this change is only for if a node is configured with `-i2pacceptincoming=0` to not accept inbound I2P connections?
💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2107774051)
> but we'd have to change the binary format here to skip over the empty 0 entry for the coinbase?
Yes - I'll adapt this PR to return the undo data from storage (with as little overhead as possible).
Indeed, the binary format will be `vector<vector<Coin>>`[^1], for the all block's transactions (except the first one).
[^1]: https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/src/undo.h#L51-L68
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2107774051)
> but we'd have to change the binary format here to skip over the empty 0 entry for the coinbase?
Yes - I'll adapt this PR to return the undo data from storage (with as little overhead as possible).
Indeed, the binary format will be `vector<vector<Coin>>`[^1], for the all block's transactions (except the first one).
[^1]: https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/src/undo.h#L51-L68
💬 DanGould commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#issuecomment-2910492898)
The main reason for the Bitcoin Core wallet to support payjoin directly is to reduce the number of necessary dependencies for those already using the core wallet. In particular rust-payjoin requires TLS and rust implementations of cryptographic primatives that core could avoid with a bespoke implementation.
(https://github.com/bitcoin/bitcoin/pull/32617#issuecomment-2910492898)
The main reason for the Bitcoin Core wallet to support payjoin directly is to reduce the number of necessary dependencies for those already using the core wallet. In particular rust-payjoin requires TLS and rust implementations of cryptographic primatives that core could avoid with a bespoke implementation.
📝 romanz converted_to_draft a pull request: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540)
Today, it is possible to fetch a block's spent prevouts in order to build an external index by using the `/rest/block/BLOCKHASH.json` endpoint. However, its performance is low due to JSON serialization overhead.
We can significantly optimize it by adding a new [REST API](https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md) endpoint, using a binary response format (returning a collection of spent txout lists, one per each block transaction):
```
$ BLOCKHASH=000000000000000
...
(https://github.com/bitcoin/bitcoin/pull/32540)
Today, it is possible to fetch a block's spent prevouts in order to build an external index by using the `/rest/block/BLOCKHASH.json` endpoint. However, its performance is low due to JSON serialization overhead.
We can significantly optimize it by adding a new [REST API](https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md) endpoint, using a binary response format (returning a collection of spent txout lists, one per each block transaction):
```
$ BLOCKHASH=000000000000000
...
📝 achow101 converted_to_draft a pull request: "wallet: Remove watchonly behavior and isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523)
Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet's internals.
Additionally, ISMINE_USED is removed as its use is only as a filter for cached balances. This PR changes the caching to utilize bools
...
(https://github.com/bitcoin/bitcoin/pull/32523)
Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet's internals.
Additionally, ISMINE_USED is removed as its use is only as a filter for cached balances. This PR changes the caching to utilize bools
...
📝 achow101 opened a pull request: "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs"
(https://github.com/bitcoin/bitcoin/pull/32618)
Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet's internals.
With all of the watchonly things removed, ISMINE_WATCH_ONLY is removed as well.
Split from #32523
Depends on #32594 for tests th
...
(https://github.com/bitcoin/bitcoin/pull/32618)
Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet's internals.
With all of the watchonly things removed, ISMINE_WATCH_ONLY is removed as well.
Split from #32523
Depends on #32594 for tests th
...
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107792280)
I've added a test and a fix for this in #32618
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107792280)
I've added a test and a fix for this in #32618
💬 ismaelsadeeq commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2107794067)
I'll prefer the approach I suggested.
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2107794067)
I'll prefer the approach I suggested.
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107795475)
Added in #32618
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107795475)
Added in #32618
💬 brunoerg commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2910538498)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2910538498)
Concept ACK
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808053)
Done in #32618
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808053)
Done in #32618
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808368)
Done
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808368)
Done
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808434)
Done
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808434)
Done
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808508)
Done
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808508)
Done
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808623)
Done
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808623)
Done
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808659)
Done
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808659)
Done