🚀 achow101 merged a pull request: "fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target"
(https://github.com/bitcoin/bitcoin/pull/30826)
(https://github.com/bitcoin/bitcoin/pull/30826)
💬 davidgumberg commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2364259064)
reACK https://github.com/bitcoin/bitcoin/commit/9967e8feb62a0c30612e437b70d1d64a2e67b9e0
Thanks for putting up with the review churn on this, it looks great! This PR makes peer disconnect message logging more consistent, and makes us better respect the `-logips` argument.
I reviewed the range-diff:
```bash
git range-diff a74bdeea..08cd29f5 6fc46927..9967e8fe
```
<details>
<summary>Manual testing</summary>
Starting a node and then doing `bitcoin-cli setnetworkactive false` and
...
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2364259064)
reACK https://github.com/bitcoin/bitcoin/commit/9967e8feb62a0c30612e437b70d1d64a2e67b9e0
Thanks for putting up with the review churn on this, it looks great! This PR makes peer disconnect message logging more consistent, and makes us better respect the `-logips` argument.
I reviewed the range-diff:
```bash
git range-diff a74bdeea..08cd29f5 6fc46927..9967e8fe
```
<details>
<summary>Manual testing</summary>
Starting a node and then doing `bitcoin-cli setnetworkactive false` and
...
💬 achow101 commented on pull request "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work":
(https://github.com/bitcoin/bitcoin/pull/30918#issuecomment-2364294764)
ACK 284bd17309ab3b124d9dcddfec62f5506383343b
(https://github.com/bitcoin/bitcoin/pull/30918#issuecomment-2364294764)
ACK 284bd17309ab3b124d9dcddfec62f5506383343b
🚀 achow101 merged a pull request: "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work"
(https://github.com/bitcoin/bitcoin/pull/30918)
(https://github.com/bitcoin/bitcoin/pull/30918)
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#issuecomment-2364331594)
> Is this still the case? If not, please update the OP.
Removed, thank you.
(https://github.com/bitcoin/bitcoin/pull/30765#issuecomment-2364331594)
> Is this still the case? If not, please update the OP.
Removed, thank you.
💬 achow101 commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#issuecomment-2364351304)
ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7
(https://github.com/bitcoin/bitcoin/pull/30765#issuecomment-2364351304)
ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7
🚀 achow101 merged a pull request: "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors"
(https://github.com/bitcoin/bitcoin/pull/30765)
(https://github.com/bitcoin/bitcoin/pull/30765)
💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2364509676)
ping review @maflcko @marcofleon
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2364509676)
ping review @maflcko @marcofleon
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1769185572)
Ugh yes, a bad rebase sorry. I've been using a new setup and haven't perfected my diff-viewing yet.
I don't understand how 0 tests failed, though 😬 gotta look into that
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1769185572)
Ugh yes, a bad rebase sorry. I've been using a new setup and haven't perfected my diff-viewing yet.
I don't understand how 0 tests failed, though 😬 gotta look into that
💬 mzumsande commented on issue "Intermittent failure in p2p_1p1c_network.py", line 58, in raise_network_minfee assert_greater_than(node.getmempoolinfo()['mempoolminfee'], FEERATE_1SAT_VB) ; AssertionError: 0.00001000 <= 0.00001000":
(https://github.com/bitcoin/bitcoin/issues/30922#issuecomment-2364529013)
I think that the problem is in how `fill_mempool` works with multiple nodes: If `node0` submits transactions so fast that they are being evicted from the mempool before they could be sent out to other peers, then the other peers will never see them, admit (and later remove) them from their mempool and therefore won't raise their minfee.
Not sure if this can be fixed within `p2p_1p1c_network.py` or if some sort of intermediate sync_mempool calls would need to be added to `fill_mempool` after
...
(https://github.com/bitcoin/bitcoin/issues/30922#issuecomment-2364529013)
I think that the problem is in how `fill_mempool` works with multiple nodes: If `node0` submits transactions so fast that they are being evicted from the mempool before they could be sent out to other peers, then the other peers will never see them, admit (and later remove) them from their mempool and therefore won't raise their minfee.
Not sure if this can be fixed within `p2p_1p1c_network.py` or if some sort of intermediate sync_mempool calls would need to be added to `fill_mempool` after
...
⚠️ kegdeg opened an issue: "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password"
(https://github.com/bitcoin/bitcoin/issues/30939)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
After providing rpc credentials using bitcoin-cli, the following error appears:
'Error parsing command line arguments: Invalid parameter -rpcpasssword=password'
telnet to localhost connects with bitcoind running
### Expected behaviour
no error
### Steps to reproduce
This is how the bitcoin.conf looks like:
# Enable RPC server
server=1
# RPC settings
rpcuser=user
rpcpassw
...
(https://github.com/bitcoin/bitcoin/issues/30939)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
After providing rpc credentials using bitcoin-cli, the following error appears:
'Error parsing command line arguments: Invalid parameter -rpcpasssword=password'
telnet to localhost connects with bitcoind running
### Expected behaviour
no error
### Steps to reproduce
This is how the bitcoin.conf looks like:
# Enable RPC server
server=1
# RPC settings
rpcuser=user
rpcpassw
...
💬 fjahr commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#issuecomment-2364572949)
Addressed feedback from @ismaelsadeeq and @furszy , fixing logs and adding more test coverage.
(https://github.com/bitcoin/bitcoin/pull/30678#issuecomment-2364572949)
Addressed feedback from @ismaelsadeeq and @furszy , fixing logs and adding more test coverage.
💬 pinheadmz commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2364602199)
"rpcpasssword" with 3 S's or 2?
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2364602199)
"rpcpasssword" with 3 S's or 2?
💬 bitcoin-tools commented on issue "`test_bitcoin` from pre-built 28.0rc2 tarball is failing for JSON parsing":
(https://github.com/bitcoin/bitcoin/issues/30938#issuecomment-2364612659)
Researching it deeper, it may be better to wrap the command in `sh -c '...'`, since that would run `echo` and `false` as shell built-ins, rather than depending on coreutils binaries. But I agree with your reasoning regarding `ls`.
(https://github.com/bitcoin/bitcoin/issues/30938#issuecomment-2364612659)
Researching it deeper, it may be better to wrap the command in `sh -c '...'`, since that would run `echo` and `false` as shell built-ins, rather than depending on coreutils binaries. But I agree with your reasoning regarding `ls`.
🤔 glozow reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2317959224)
Read through the code and it makes sense to me, but haven't tested much. It would be nice to have a fuzz coverage report? @dergoegge
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2317959224)
Read through the code and it makes sense to me, but haven't tested much. It would be nice to have a fuzz coverage report? @dergoegge
💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1768420948)
nit in 40224019cd6e8259118412529c554d8b4a260130
```suggestion
// Parents of a transaction do not have ancestors inside those parents (except itself).
```
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1768420948)
nit in 40224019cd6e8259118412529c554d8b4a260130
```suggestion
// Parents of a transaction do not have ancestors inside those parents (except itself).
```
💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1769251003)
Ok, so iiuc, if you add + remove + add + remove the same number of transactions, the depgraph isn't going to get larger and larger; the new transactions can take the place of the holes.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1769251003)
Ok, so iiuc, if you add + remove + add + remove the same number of transactions, the depgraph isn't going to get larger and larger; the new transactions can take the place of the holes.
💬 kegdeg commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2364630756)
Oh wow, it should be 2 's'. Although I also had that error when using another password which I copy/pasted (maybe there was a spacing error cannot check now), just used password for the example. Those commands and bitcoin.conf are the correct ones, right? Will check it back again when I am back in front of the terminal.
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2364630756)
Oh wow, it should be 2 's'. Although I also had that error when using another password which I copy/pasted (maybe there was a spacing error cannot check now), just used password for the example. Those commands and bitcoin.conf are the correct ones, right? Will check it back again when I am back in front of the terminal.
💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2364675182)
CI failures (timeouts) seem unrelated, should be ready for review as I am hoping https://github.com/bitcoin/bitcoin/pull/30901 will get merged quickly.
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2364675182)
CI failures (timeouts) seem unrelated, should be ready for review as I am hoping https://github.com/bitcoin/bitcoin/pull/30901 will get merged quickly.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1769352568)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1769352568)
Fixed.