✅ 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.
👍 stickies-v approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261#pullrequestreview-1401538483)
re-ACK 4eee95e57
Verified that the only changes are a refactoring to `bench`, removing now unnecessary helper functions `ResolveIP` and `ResolveService`, and incorporating my previous outstanding review comments.
(https://github.com/bitcoin/bitcoin/pull/26261#pullrequestreview-1401538483)
re-ACK 4eee95e57
Verified that the only changes are a refactoring to `bench`, removing now unnecessary helper functions `ResolveIP` and `ResolveService`, and incorporating my previous outstanding review comments.
💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1177587552)
Any reason why not just reuse the already looked up service?
```suggestion
const CService service{Lookup("250.3.1.1", 8333, false).value()};
addrman.Add({CAddress(service, NODE_NONE)}, service);
```
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1177587552)
Any reason why not just reuse the already looked up service?
```suggestion
const CService service{Lookup("250.3.1.1", 8333, false).value()};
addrman.Add({CAddress(service, NODE_NONE)}, service);
```
💬 furszy commented on pull request "index: Compare deserialized block hash with the block hash from the blockindex":
(https://github.com/bitcoin/bitcoin/pull/26390#issuecomment-1523092479)
yeah @kcalvinalvin, please rebase it. Will check it asap.
(https://github.com/bitcoin/bitcoin/pull/26390#issuecomment-1523092479)
yeah @kcalvinalvin, please rebase it. Will check it asap.
💬 brunoerg commented on pull request "test: clean up logs when there aren't perf subprocesses":
(https://github.com/bitcoin/bitcoin/pull/23928#discussion_r1177611645)
Make sense, gonna address it!
(https://github.com/bitcoin/bitcoin/pull/23928#discussion_r1177611645)
Make sense, gonna address it!