π brunoerg approved a pull request: "Move IsDeprecatedRPCEnabled to rpc/util, rm redundant rpcEnableDeprecated"
(https://github.com/bitcoin/bitcoin/pull/27322#pullrequestreview-1417299823)
crACK 9ed9b50068a3cbcf48264d814ffe73e0b9ed10b4
nit: s/librairies/libraries in 1a1f3518ed4eb2b18c63d98fbdca10a2cda8bf1e message
(https://github.com/bitcoin/bitcoin/pull/27322#pullrequestreview-1417299823)
crACK 9ed9b50068a3cbcf48264d814ffe73e0b9ed10b4
nit: s/librairies/libraries in 1a1f3518ed4eb2b18c63d98fbdca10a2cda8bf1e message
π¬ disappointed72 commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1538897495)
Same issue with 24.0.1 node today and went back to normal after daemon reload. My node do NOT run in debug mode too.
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1538897495)
Same issue with 24.0.1 node today and went back to normal after daemon reload. My node do NOT run in debug mode too.
π theuni approved a pull request: "Introduce platform-agnostic `ALWAYS_INLINE` macro"
(https://github.com/bitcoin/bitcoin/pull/27575#pullrequestreview-1417374039)
utACK 3f19875d667522412408d06873e87ff8150e49c4
(https://github.com/bitcoin/bitcoin/pull/27575#pullrequestreview-1417374039)
utACK 3f19875d667522412408d06873e87ff8150e49c4
π pinheadmz opened a pull request: "Make peer eviction slightly more aggresive to make room for whitelisted inbound connections"
(https://github.com/bitcoin/bitcoin/pull/27600)
Closes https://github.com/bitcoin/bitcoin/issues/8798
Use case: I run a full node that accepts inbound connections and have a `whitebind` setting so my personal light client can always connect, even when `maxconnections` (and particularly all inbound slots) is already full.
Currently when connections are full, if we receive in inbound peer request, we look for a current connection to evict so the new peer can have a slot. To find an evict-able peer we go through all our peers and "protect"
...
(https://github.com/bitcoin/bitcoin/pull/27600)
Closes https://github.com/bitcoin/bitcoin/issues/8798
Use case: I run a full node that accepts inbound connections and have a `whitebind` setting so my personal light client can always connect, even when `maxconnections` (and particularly all inbound slots) is already full.
Currently when connections are full, if we receive in inbound peer request, we look for a current connection to evict so the new peer can have a slot. To find an evict-able peer we go through all our peers and "protect"
...
π¬ pinheadmz commented on issue "whiteconnections should be re-added":
(https://github.com/bitcoin/bitcoin/issues/8798#issuecomment-1538967531)
Possible solution WIP: https://github.com/bitcoin/bitcoin/pull/27600 looking for concept ACKs !
(https://github.com/bitcoin/bitcoin/issues/8798#issuecomment-1538967531)
Possible solution WIP: https://github.com/bitcoin/bitcoin/pull/27600 looking for concept ACKs !
π¬ poiuty commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1538980020)
I have the same problem. `bitcoind` started using 100% cpu (single thread) on my servers. I used the binary from https://bitcoincore.org/bin/bitcoin-core-24.0.1/bitcoin-24.0.1-x86_64-linux-gnu.tar.gz
<img src="https://img.poiuty.com/a/3c/c68204e690ae2d605cfd60e03eee0f3c.jpg">
Built bitcoind from source (v24.0.1.tar.gz). Works fine for an hour now.
```
./configure --without-gui --disable-zmq --disable-wallet
```
<img src="https://img.poiuty.com/a/7b/5234d849fcf5248c4a1adaf5011bc57b.jp
...
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1538980020)
I have the same problem. `bitcoind` started using 100% cpu (single thread) on my servers. I used the binary from https://bitcoincore.org/bin/bitcoin-core-24.0.1/bitcoin-24.0.1-x86_64-linux-gnu.tar.gz
<img src="https://img.poiuty.com/a/3c/c68204e690ae2d605cfd60e03eee0f3c.jpg">
Built bitcoind from source (v24.0.1.tar.gz). Works fine for an hour now.
```
./configure --without-gui --disable-zmq --disable-wallet
```
<img src="https://img.poiuty.com/a/7b/5234d849fcf5248c4a1adaf5011bc57b.jp
...
π¬ grubles commented on issue "make check errors on big endian OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-1538987958)
Ok I have access to the POWER9 machine I was testing with before now. I have FreeBSD 13.2 powerpc64 running on it at the moment with Bitcoin Core v25.0rc1.
Running the tests with `test/functional/test_runner.py --extended` ended up with quite a few failing:
```
feature_addrman.py | β Failed | 1 s
feature_rbf.py | β Failed | 7 s
feature_segwit.py --legacy-wallet | β Failed | 8 s
rpc_psbt
...
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-1538987958)
Ok I have access to the POWER9 machine I was testing with before now. I have FreeBSD 13.2 powerpc64 running on it at the moment with Bitcoin Core v25.0rc1.
Running the tests with `test/functional/test_runner.py --extended` ended up with quite a few failing:
```
feature_addrman.py | β Failed | 1 s
feature_rbf.py | β Failed | 7 s
feature_segwit.py --legacy-wallet | β Failed | 8 s
rpc_psbt
...
π¬ golden-guy commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1539007849)
> Same issue with 24.0.1 node today and went back to normal after daemon reload. My node do NOT run in debug mode too.
Same here using the 24.0.1 binaries NOT running in debug mode since yesterday. (Binaries from https://bitcoincore.org/bin/bitcoin-core-24.0.1/bitcoin-24.0.1-x86_64-linux-gnu.tar.gz) I didn't restart the node, but used the setban approach as pointed out here https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537784340
Since then, the single core load has decrea
...
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1539007849)
> Same issue with 24.0.1 node today and went back to normal after daemon reload. My node do NOT run in debug mode too.
Same here using the 24.0.1 binaries NOT running in debug mode since yesterday. (Binaries from https://bitcoincore.org/bin/bitcoin-core-24.0.1/bitcoin-24.0.1-x86_64-linux-gnu.tar.gz) I didn't restart the node, but used the setban approach as pointed out here https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1537784340
Since then, the single core load has decrea
...
π¬ instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1187734505)
this comment becomes a bit disconnected from the logic itself.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1187734505)
this comment becomes a bit disconnected from the logic itself.
π€ instagibbs reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1417261502)
Last bit of nitting:
`AcceptPackage` -> `AcceptAncestorPackage`
`AcceptMultipleTransactions` -> `AcceptSubPackage`
With BIP331 I think maybe knowing what an ancestor package is and how it fits
into this code may become clearer?
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1417261502)
Last bit of nitting:
`AcceptPackage` -> `AcceptAncestorPackage`
`AcceptMultipleTransactions` -> `AcceptSubPackage`
With BIP331 I think maybe knowing what an ancestor package is and how it fits
into this code may become clearer?
π¬ instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1187848024)
I know what this means, but kinda... odd that `AncestorPackage` can be `!IsAncestorPackage`
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1187848024)
I know what this means, but kinda... odd that `AncestorPackage` can be `!IsAncestorPackage`
π¬ instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1187856431)
should just leave this example in in the positive sense of it being accepted?
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1187856431)
should just leave this example in in the positive sense of it being accepted?
π¬ TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187890064)
Adding `Assert(false)` seems to throw an error:
```
util/chaintype.cpp: In function βstd::string ChainTypeToString(ChainType)β:
./util/check.h:73:89: error: control reaches end of non-void function [-Werror=return-type]
73 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
| ^
util/chaintype.cpp:23:5: note: in expansion of macro βAssertβ
23 | Ass
...
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187890064)
Adding `Assert(false)` seems to throw an error:
```
util/chaintype.cpp: In function βstd::string ChainTypeToString(ChainType)β:
./util/check.h:73:89: error: control reaches end of non-void function [-Werror=return-type]
73 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
| ^
util/chaintype.cpp:23:5: note: in expansion of macro βAssertβ
23 | Ass
...
π furszy opened a pull request: "wallet: don't duplicate change output if already exist"
(https://github.com/bitcoin/bitcoin/pull/27601)
If the transaction includes an output that goes to the custom change
address (`CoinControl::destChange`), the transaction creation process
should avoid creating duplicate outputs to the same address.
Instead, it should reuse the existing output to prevent revealing
which address is the change address, which could compromise privacy.
Side note:
This is something that will be using for the #26732 rework.
(https://github.com/bitcoin/bitcoin/pull/27601)
If the transaction includes an output that goes to the custom change
address (`CoinControl::destChange`), the transaction creation process
should avoid creating duplicate outputs to the same address.
Instead, it should reuse the existing output to prevent revealing
which address is the change address, which could compromise privacy.
Side note:
This is something that will be using for the #26732 rework.
π sr-gi opened a pull request: "net: avoid serving non-announced txs as a result of a MEMPOOL message"
(https://github.com/bitcoin/bitcoin/pull/27602)
Currently, non-announced transactions can be served after a BIP35 MEMPOOl message has been received from a peer. By following this pattern, we are potentially allowing peers to fingerprint transactions that may have originated in our node and to perform timing analysis on the propagation of transactions that enter our mempool.
This simply removes the relay bypass so only announced transactions are served to peers.
(https://github.com/bitcoin/bitcoin/pull/27602)
Currently, non-announced transactions can be served after a BIP35 MEMPOOl message has been received from a peer. By following this pattern, we are potentially allowing peers to fingerprint transactions that may have originated in our node and to perform timing analysis on the propagation of transactions that enter our mempool.
This simply removes the relay bypass so only announced transactions are served to peers.
π¬ sr-gi commented on pull request "net: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1539074312)
Motivation: this fix was discussed over IRC by @willcl-ark @glozow @sipa and @darosior
https://gnusha.org/bitcoin-core-dev/2023-05-02.log
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1539074312)
Motivation: this fix was discussed over IRC by @willcl-ark @glozow @sipa and @darosior
https://gnusha.org/bitcoin-core-dev/2023-05-02.log
π¬ TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1539094705)
Thank you for the reviews!
Updated 95744f9cf1f143b6449a5046a914557b3886e3a2 -> 43dec8806f679005d4484b229f433687526c9133 ([kernelChainType_4](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_4) -> [kernelChainType_5](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainType_4..kernelChainType_5))
* Addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187440836),
...
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1539094705)
Thank you for the reviews!
Updated 95744f9cf1f143b6449a5046a914557b3886e3a2 -> 43dec8806f679005d4484b229f433687526c9133 ([kernelChainType_4](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_4) -> [kernelChainType_5](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainType_4..kernelChainType_5))
* Addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187440836),
...
π¬ desirepl commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1539136159)
@pinheadmz
2023-05-08T22:11:59Z Using data directory W:\BitcoinCore\_strDataDir (this was read from hive)
2023-05-08T22:11:59Z Config file: W:\BitcoinCore\_strDataDir\bitcoin.conf (exist...)
2023-05-08T22:11:59Z Config file arg: blocksdir="W:/BitcoinCore/BlocksDir_/_.no_params"
2023-05-08T22:11:59Z Config file arg: datadir="W:/BitcoinCore/DataDir_/_.no_params" (this is in 2nd line bitcoin.conf, *ignored*)
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1539136159)
@pinheadmz
2023-05-08T22:11:59Z Using data directory W:\BitcoinCore\_strDataDir (this was read from hive)
2023-05-08T22:11:59Z Config file: W:\BitcoinCore\_strDataDir\bitcoin.conf (exist...)
2023-05-08T22:11:59Z Config file arg: blocksdir="W:/BitcoinCore/BlocksDir_/_.no_params"
2023-05-08T22:11:59Z Config file arg: datadir="W:/BitcoinCore/DataDir_/_.no_params" (this is in 2nd line bitcoin.conf, *ignored*)
π furszy approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1417686210)
diff ACK 265b9e3
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1417686210)
diff ACK 265b9e3
π kevkevinpal opened a pull request: "test: added coverage to mining_basic.py"
(https://github.com/bitcoin/bitcoin/pull/27603)
Included a test that checks if the block.vtx.empty() does it throw the correct error, currently we only check if !block.vtx->IsCoinBase()
I've tested to make sure this actually doing what I intended by breaking up this if block into two if blocks with different error messages and running the functional test
https://github.com/bitcoin/bitcoin/blob/322ec63b01499c1ec52d3912ee382ebd59f2366b/src/rpc/mining.cpp#L963
This change should increase the test coverage for the `submitblock()` rpc in `.
...
(https://github.com/bitcoin/bitcoin/pull/27603)
Included a test that checks if the block.vtx.empty() does it throw the correct error, currently we only check if !block.vtx->IsCoinBase()
I've tested to make sure this actually doing what I intended by breaking up this if block into two if blocks with different error messages and running the functional test
https://github.com/bitcoin/bitcoin/blob/322ec63b01499c1ec52d3912ee382ebd59f2366b/src/rpc/mining.cpp#L963
This change should increase the test coverage for the `submitblock()` rpc in `.
...