💬 laanwj commented on pull request "doc: Fix gen-manpages to check build options":
(https://github.com/bitcoin/bitcoin/pull/29457#issuecomment-2414143198)
Thanks for working on this issue, however this needs update after the cmake change.. This is the new contents of `test/config.ini`:
```ini
[components]
# Which components are enabled. These are commented out by `configure` if they were disabled when running config.
ENABLE_WALLET=true
USE_SQLITE=true
#USE_BDB=true
ENABLE_CLI=true
ENABLE_BITCOIN_UTIL=true
ENABLE_WALLET_TOOL=true
ENABLE_BITCOIND=true
#ENABLE_FUZZ_BINARY=true
#ENABLE_ZMQ=true
ENABLE_EXTERNAL_SIGNER=true
#ENABLE_USDT_TR
...
(https://github.com/bitcoin/bitcoin/pull/29457#issuecomment-2414143198)
Thanks for working on this issue, however this needs update after the cmake change.. This is the new contents of `test/config.ini`:
```ini
[components]
# Which components are enabled. These are commented out by `configure` if they were disabled when running config.
ENABLE_WALLET=true
USE_SQLITE=true
#USE_BDB=true
ENABLE_CLI=true
ENABLE_BITCOIN_UTIL=true
ENABLE_WALLET_TOOL=true
ENABLE_BITCOIND=true
#ENABLE_FUZZ_BINARY=true
#ENABLE_ZMQ=true
ENABLE_EXTERNAL_SIGNER=true
#ENABLE_USDT_TR
...
✅ achow101 closed a pull request: "RPC/Wallet: Add "use_txids" to output of getaddressinfo"
(https://github.com/bitcoin/bitcoin/pull/22693)
(https://github.com/bitcoin/bitcoin/pull/22693)
💬 achow101 commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-2414143984)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-2414143984)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
✅ achow101 closed a pull request: "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type"
(https://github.com/bitcoin/bitcoin/pull/29748)
(https://github.com/bitcoin/bitcoin/pull/29748)
💬 achow101 commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2414146220)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2414146220)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
✅ achow101 closed a pull request: "BufferedFile: fclose at destruction"
(https://github.com/bitcoin/bitcoin/pull/29614)
(https://github.com/bitcoin/bitcoin/pull/29614)
💬 achow101 commented on pull request "BufferedFile: fclose at destruction":
(https://github.com/bitcoin/bitcoin/pull/29614#issuecomment-2414154943)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/29614#issuecomment-2414154943)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
💬 achow101 commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2414160275)
Please rebase for cmake if you are still interested in working on this.
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2414160275)
Please rebase for cmake if you are still interested in working on this.
💬 achow101 commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#issuecomment-2414164245)
Is this still relevant after cmake?
(https://github.com/bitcoin/bitcoin/pull/30136#issuecomment-2414164245)
Is this still relevant after cmake?
💬 maflcko commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-2414166896)
CI fails, presumably after https://github.com/bitcoin/bitcoin/pull/29071
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-2414166896)
CI fails, presumably after https://github.com/bitcoin/bitcoin/pull/29071
💬 TheCharlatan commented on pull request "validation: Make ReplayBlocks interruptible":
(https://github.com/bitcoin/bitcoin/pull/30155#issuecomment-2414167167)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30155#issuecomment-2414167167)
Concept ACK
💬 l0rinc commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2414178926)
> My suggestion was for @DrahtBot to add the label only once it had actually run the scripted-diff successfully.
If you think that's better than doing it in a separate job, I'll investigate.
@maflcko, would that work with you as well?
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2414178926)
> My suggestion was for @DrahtBot to add the label only once it had actually run the scripted-diff successfully.
If you think that's better than doing it in a separate job, I'll investigate.
@maflcko, would that work with you as well?
💬 maflcko commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#issuecomment-2414179165)
For reference, CMake has this documented as `set(CMAKE_CXX_STANDARD 20)` and also fails if it is violated.
(https://github.com/bitcoin/bitcoin/pull/30136#issuecomment-2414179165)
For reference, CMake has this documented as `set(CMAKE_CXX_STANDARD 20)` and also fails if it is violated.
📝 ryanofsky converted_to_draft a pull request: "wallet: Avoid potentially writing incorrect best block locator"
(https://github.com/bitcoin/bitcoin/pull/29652)
This PR updates the `CWallet::chainStateFlushed` method to write the `m_last_block_processed` locator to the database as the wallet's best block instead of the chain tip locator.
Saving the last block processed locator as the best block is less fragile than saving `chainStateFlushed` locator as best block.
Right now wallet code relies on `blockConnected` notifications being sent before `chainStateFlushed` notifications, and on the `chainStateFlushed` locator pointing to the last connected
...
(https://github.com/bitcoin/bitcoin/pull/29652)
This PR updates the `CWallet::chainStateFlushed` method to write the `m_last_block_processed` locator to the database as the wallet's best block instead of the chain tip locator.
Saving the last block processed locator as the best block is less fragile than saving `chainStateFlushed` locator as best block.
Right now wallet code relies on `blockConnected` notifications being sent before `chainStateFlushed` notifications, and on the `chainStateFlushed` locator pointing to the last connected
...
💬 l0rinc commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1801361386)
I did see that conversation, but I don't think it's the benchmark's responsibility to reveal the internals of a certain group of tests.
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1801361386)
I did see that conversation, but I don't think it's the benchmark's responsibility to reveal the internals of a certain group of tests.
✅ achow101 closed a pull request: "Remove redundant `-datacarrier` option"
(https://github.com/bitcoin/bitcoin/pull/29942)
(https://github.com/bitcoin/bitcoin/pull/29942)
💬 achow101 commented on pull request "Remove redundant `-datacarrier` option":
(https://github.com/bitcoin/bitcoin/pull/29942#issuecomment-2414187917)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/29942#issuecomment-2414187917)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
💬 stickies-v commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1801362597)
Oh would also add that deprecated behaviour should be documented in the RPC docs, such as e.g. in https://github.com/bitcoin/bitcoin/blob/0ca1d1bf69ca364393e924cf41becfde1b68126c/src/rpc/net.cpp#L663-L664?
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1801362597)
Oh would also add that deprecated behaviour should be documented in the RPC docs, such as e.g. in https://github.com/bitcoin/bitcoin/blob/0ca1d1bf69ca364393e924cf41becfde1b68126c/src/rpc/net.cpp#L663-L664?
💬 stickies-v commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1801361376)
Perhaps useful to elaborate a bit more on distinguishing between backwards compatible/incompatible behaviour?
```suggestion
- Implement an associated `-deprecatedrpc=` option to retain previous RPC behavior when modifying the structure of RPC interface JSON in a backwards-incompatible manner. This includes, but is not limited to, changes such as: data types changes (e.g. from `{"result":{"warnings":""}}` to `{"result":{"warnings":[]}}`, changing a value from a string to a number, etc.), log
...
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1801361376)
Perhaps useful to elaborate a bit more on distinguishing between backwards compatible/incompatible behaviour?
```suggestion
- Implement an associated `-deprecatedrpc=` option to retain previous RPC behavior when modifying the structure of RPC interface JSON in a backwards-incompatible manner. This includes, but is not limited to, changes such as: data types changes (e.g. from `{"result":{"warnings":""}}` to `{"result":{"warnings":[]}}`, changing a value from a string to a number, etc.), log
...
🤔 stickies-v reviewed a pull request: "doc: add guidance for RPC to developer notes"
(https://github.com/bitcoin/bitcoin/pull/30142#pullrequestreview-2369659601)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30142#pullrequestreview-2369659601)
Concept ACK