📝 achow101 converted_to_draft a pull request: "refactor: Remove settings merge reverse precedence code"
(https://github.com/bitcoin/bitcoin/pull/17581)
**This is based on #16545 + #17580 + #17493.** The non-base commits are:
- [`a057e0c38f2` refactor: Remove settings merge reverse precedence code](https://github.com/bitcoin/bitcoin/pull/17581/commits/a057e0c38f2d56219568bc6f4ff9e972cdb339c0)
---
This has no effect on behavior because as of https://github.com/bitcoin/bitcoin/pull/17493 it's not possible to specify multiple values for single value settings in the config file.
This change implements one of the settings simplifications
...
(https://github.com/bitcoin/bitcoin/pull/17581)
**This is based on #16545 + #17580 + #17493.** The non-base commits are:
- [`a057e0c38f2` refactor: Remove settings merge reverse precedence code](https://github.com/bitcoin/bitcoin/pull/17581/commits/a057e0c38f2d56219568bc6f4ff9e972cdb339c0)
---
This has no effect on behavior because as of https://github.com/bitcoin/bitcoin/pull/17493 it's not possible to specify multiple values for single value settings in the config file.
This change implements one of the settings simplifications
...
📝 achow101 converted_to_draft a pull request: "fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses"
(https://github.com/bitcoin/bitcoin/pull/26859)
In the process of doing so, refactor `ConsumeNetAddr()` to generate the addresses from IPv4, IPv6, Tor, I2P and CJDNS networks in the same way - by preparing some random stream and deserializing from it. Similar code was already found in `RandAddr()`.
(https://github.com/bitcoin/bitcoin/pull/26859)
In the process of doing so, refactor `ConsumeNetAddr()` to generate the addresses from IPv4, IPv6, Tor, I2P and CJDNS networks in the same way - by preparing some random stream and deserializing from it. Similar code was already found in `RandAddr()`.
💬 achow101 commented on pull request "wallet: Refactor and document CoinControl":
(https://github.com/bitcoin/bitcoin/pull/26066#issuecomment-1522073245)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/26066#issuecomment-1522073245)
Are you still working on this?
💬 ekzyis commented on issue "Consider Removing Message Signing":
(https://github.com/bitcoin/bitcoin/issues/27515#issuecomment-1522078187)
> Would arguably be better to do such checks by signing a transaction and not broadcasting it.
I think having to create a transaction is unnecessary complex if you just want to check if you can spend a UTXO.
For a transaction, you need to specify amount, fee rate and receiving address. And after signing, you must make sure to wipe this transaction completely from your computer so no one else can broadcast it.
> You don't fight bullies with hiding and give up things. You push back like peo
...
(https://github.com/bitcoin/bitcoin/issues/27515#issuecomment-1522078187)
> Would arguably be better to do such checks by signing a transaction and not broadcasting it.
I think having to create a transaction is unnecessary complex if you just want to check if you can spend a UTXO.
For a transaction, you need to specify amount, fee rate and receiving address. And after signing, you must make sure to wipe this transaction completely from your computer so no one else can broadcast it.
> You don't fight bullies with hiding and give up things. You push back like peo
...
✅ achow101 closed a pull request: "rpc/wallet: Add details and duplicate section for simulaterawtransaction"
(https://github.com/bitcoin/bitcoin/pull/25621)
(https://github.com/bitcoin/bitcoin/pull/25621)
💬 achow101 commented on pull request "wallet: Guard against undefined behaviour":
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1522082927)
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1522082927)
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.
✅ achow101 closed a pull request: "wallet: Guard against undefined behaviour"
(https://github.com/bitcoin/bitcoin/pull/25982)
(https://github.com/bitcoin/bitcoin/pull/25982)
💬 achow101 commented on pull request "refactor: make some BlockManager members const":
(https://github.com/bitcoin/bitcoin/pull/26664#issuecomment-1522083768)
The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.
Closing due to lack of interest. Pull requests with improvements are always welcome.
(https://github.com/bitcoin/bitcoin/pull/26664#issuecomment-1522083768)
The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.
Closing due to lack of interest. Pull requests with improvements are always welcome.
✅ achow101 closed a pull request: "refactor: make some BlockManager members const"
(https://github.com/bitcoin/bitcoin/pull/26664)
(https://github.com/bitcoin/bitcoin/pull/26664)
💬 achow101 commented on pull request "refactor: replace all implicit C-style const/const+reinterpret with explicit casts":
(https://github.com/bitcoin/bitcoin/pull/27126#issuecomment-1522085388)
This PR does not seem to have conceptual support. Please leave a comment if you would like this to be reopened.
(https://github.com/bitcoin/bitcoin/pull/27126#issuecomment-1522085388)
This PR does not seem to have conceptual support. Please leave a comment if you would like this to be reopened.
✅ achow101 closed a pull request: "refactor: replace all implicit C-style const/const+reinterpret with explicit casts"
(https://github.com/bitcoin/bitcoin/pull/27126)
(https://github.com/bitcoin/bitcoin/pull/27126)
💬 ekzyis commented on pull request "doc: Improve documentation of rpcallowip":
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1176819779)
nit:
I would use "or" after "a network/netmask (e.g. 1.2.3.4/255.255.255.0)" now:
```diff
- argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24, 0.0.0.0/0 (all ipv4) or ::/0 (all ipv6)). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+ argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RP
...
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1176819779)
nit:
I would use "or" after "a network/netmask (e.g. 1.2.3.4/255.255.255.0)" now:
```diff
- argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24, 0.0.0.0/0 (all ipv4) or ::/0 (all ipv6)). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+ argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RP
...
💬 satsie commented on issue "Add per message stats to getnettotals rpc":
(https://github.com/bitcoin/bitcoin/issues/26337#issuecomment-1522205393)
Thanks @vasild! I think `show_only` is really clear and straightforward. I will update my code to use that instead.
In the process of updating, I noticed the phrase "aggregated by" used in the context of the getpeerinfo RPC, [where it breaks results down by message type](https://github.com/bitcoin/bitcoin/blob/v25.0rc1/src/rpc/net.cpp#L152):
```
{RPCResult::Type::OBJ_DYN, "bytessent_per_msg", "",
{
{RPCResult::Type::NUM, "m
...
(https://github.com/bitcoin/bitcoin/issues/26337#issuecomment-1522205393)
Thanks @vasild! I think `show_only` is really clear and straightforward. I will update my code to use that instead.
In the process of updating, I noticed the phrase "aggregated by" used in the context of the getpeerinfo RPC, [where it breaks results down by message type](https://github.com/bitcoin/bitcoin/blob/v25.0rc1/src/rpc/net.cpp#L152):
```
{RPCResult::Type::OBJ_DYN, "bytessent_per_msg", "",
{
{RPCResult::Type::NUM, "m
...
💬 TheCharlatan commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1522212627)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1522212627)
Concept ACK
💬 michaelfolkson commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1522312598)
Concept ACK
Perhaps the Bitcoin Design [community](https://bitcoin.design/) will have a view on the wording. I'd go with standalone "signet" but I haven't done any user research on whether this isn't sufficiently informative.
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1522312598)
Concept ACK
Perhaps the Bitcoin Design [community](https://bitcoin.design/) will have a view on the wording. I'd go with standalone "signet" but I haven't done any user research on whether this isn't sufficiently informative.
💬 john-moffett commented on pull request "util: Show descriptive error messages when FileCommit fails":
(https://github.com/bitcoin/bitcoin/pull/26654#issuecomment-1522357171)
Thanks for reminding me! Will rebase tomorrow.
(https://github.com/bitcoin/bitcoin/pull/26654#issuecomment-1522357171)
Thanks for reminding me! Will rebase tomorrow.
📝 Empact opened a pull request: "Remove now-unnecessary poll, fcntl includes from net(base).cpp"
(https://github.com/bitcoin/bitcoin/pull/27530)
As far as I can tell, the code calling for these includes was removed in:
6e68ccbefea6509c61fc4405a391a517c6057bb0 #24356
82d360b5a88d9057b6c09b61cd69e426c7a2412d #21387
(https://github.com/bitcoin/bitcoin/pull/27530)
As far as I can tell, the code calling for these includes was removed in:
6e68ccbefea6509c61fc4405a391a517c6057bb0 #24356
82d360b5a88d9057b6c09b61cd69e426c7a2412d #21387
💬 kcalvinalvin commented on pull request "index: Compare deserialized block hash with the block hash from the blockindex":
(https://github.com/bitcoin/bitcoin/pull/26390#issuecomment-1522778737)
> Are you still working on this?
Is there any interest for the PR itself? It's not much but you can get additional assurance that the blockfilter stored on disk is not corrupted.
I can rebase if there's interest in that feature.
(https://github.com/bitcoin/bitcoin/pull/26390#issuecomment-1522778737)
> Are you still working on this?
Is there any interest for the PR itself? It's not much but you can get additional assurance that the blockfilter stored on disk is not corrupted.
I can rebase if there's interest in that feature.
💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1523037238)
I had to touch `bench` code to fix an error CI pointed out, so I took the moment to address some suggestions from @stickies-v.
Addressed:
https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153145784
https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153157545
https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153170805
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1523037238)
I had to touch `bench` code to fix an error CI pointed out, so I took the moment to address some suggestions from @stickies-v.
Addressed:
https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153145784
https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153157545
https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153170805
💬 ajtowns commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1523087063)
Concept ACK.
Might be good to have some way of being able to configure where to connect to for sensitive relay, defaulting to tor/i2p, but so that you could perhaps enable specific ipv4/ipv6 addresses as well, particularly for testing purposes.
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1523087063)
Concept ACK.
Might be good to have some way of being able to configure where to connect to for sensitive relay, defaulting to tor/i2p, but so that you could perhaps enable specific ipv4/ipv6 addresses as well, particularly for testing purposes.