💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1635224220)
The purpose is for `PreRegisterPeer` to be callable with a fixed salt (via `PreRegisterPeerWithSalt`), so collisions can be tested. This is just moving it out of the `PImpl`, the effects on the external caller should be the same

⚠️ murchandamus opened an issue: "Improve description of the `filename` parameter of `loadwallet` RPC"
(https://github.com/bitcoin/bitcoin/issues/30269)
### Motivation

As the documentation for the [`loadwallet`](https://bitcoincore.org/en/doc/27.0.0/rpc/wallet/loadwallet/) RPC describes, the syntax is:

loadwallet "filename" ( load_on_startup )

And it further specifies:

Arguments:
1. filename (string, required) The wallet directory or .dat file.
…

What’s not clearly specified in the documentation is that you are to provide the filename _relative to the wallet directory_ `~/.bitcoin/regtest/wallets`. It would be grea
...

💬 marcofleon commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635236868)
Yes, `GetTime()` is also called in `SendComplete`.

> Does this mean that the time will be frozen for the duration of the test?

I believe so, unless I'm missing something. For each iteration, `g_mock_time` will be set to an integer within the range specified in `ConsumeTime`.

💬 marcofleon commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635237667)
This doesn't seem to work when I test it. I think this results in an unspecifed IPv6 address, so in `Hello()`, the sock returned from `m_control_host.Connect()` is always invalid.
https://github.com/bitcoin/bitcoin/blob/337f9d44c28b1d3563a0063a8805b418d506698d/src/netaddress.cpp#L425-L431

A possible option is to use `ConsumeService` here instead of explicitly setting the `addr`. I haven't tested it yet but would probably work.

💬 MukulKolpe commented on issue "Improve description of the `filename` parameter of `loadwallet` RPC":
(https://github.com/bitcoin/bitcoin/issues/30269#issuecomment-2161272201)
Hey @murchandamus, can I work on this issue?

💬 willcl-ark commented on issue "Improve description of the `filename` parameter of `loadwallet` RPC":
(https://github.com/bitcoin/bitcoin/issues/30269#issuecomment-2161360992)
@MukulKolpe absolutely. We don't generally assign issues in this project. Anyone is free to open a PR fixing any issue.

As @murchandamus advised in OP, please take care to:

> ... read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md?rgh-link-date=2024-06-11T17%3A09%3A29Z) before opening your pull request.

If you have any questions about the issue at hand here, or generally opening a Pull Request in this repo, feel free to ask away in here :)

💬 achow101 commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2161427580)
ACK 09ef322acc0a88a9e119f74923399598984c68f6

💬 murchandamus commented on issue "Improve description of the `filename` parameter of `loadwallet` RPC":
(https://github.com/bitcoin/bitcoin/issues/30269#issuecomment-2161452991)
Sure! For context, the following Bitcoin Stack Exchange topic was what inspired me to open the issue: https://bitcoin.stackexchange.com/q/123331/5406

🚀 achow101 merged a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830)

💬 Manuela123q commented on pull request "kernel: Run sanity checks on context construction":
(https://github.com/bitcoin/bitcoin/pull/28228#discussion_r1635384865)
Hhfft

💬 achow101 commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2161477616)
ACK 24bc46c83b39149f4845a575a82337eb46d91bdb

🚀 achow101 merged a pull request: "cli: Detect port errors in rpcconnect and rpcport"
(https://github.com/bitcoin/bitcoin/pull/29521)

💬 achow101 commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#issuecomment-2161560686)
ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8

💬 mzumsande commented on pull request "p2p: For assumeutxo, download snapshot chain before background chain":
(https://github.com/bitcoin/bitcoin/pull/29519#discussion_r1635456871)
> This means if the snapshot is not on the best chain, the node may be unable to sync to a better chain.

I've thought about this for a while, and I'm not sure if that problem is really being fixed by your suggestion for the following (slightly philosophical) reasons:
- Accepting a snapshot means that we've created a new chainstate with its Active Tip being set to that block.
- That is a commitment, we can't just reorg away from it to another chain that doesn't contain the snapshot block, ev
...

🚀 achow101 merged a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339)

💬 murchandamus commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#issuecomment-2161576108)
> > ```diff
> > --- a/test/functional/wallet_import_rescan.py
> > +++ b/test/functional/wallet_import_rescan.py
> > @@ -293,6 +293,7 @@ class ImportRescanTest(BitcoinTestFramework):
> > add_to_wallet=False,
> > inputs=[unspent_txid_map[variant.initial_txid]],
> > outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}],
> > + locktime=0,
> > subtract_fee_from_outputs=[0]
> > )
> >
...

👍 brunoerg approved a pull request: "fuzz: add I2P harness"
(https://github.com/bitcoin/bitcoin/pull/30230#pullrequestreview-2111463661)
ACK 193c748e44f8647a056121fc9cbb9c2efbcbfc49

💬 achow101 commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#issuecomment-2161613165)
ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883

🚀 achow101 merged a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160)

💬 Eunovo commented on pull request "Tr partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2161639208)
@ajtowns Thanks for the review. I added some examples to `wallet_miniscript.py` and updated `descriptors.md`