💬 rkrux commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1603491415)
Looking at the sample output of `subversion` - `"subversion": "\/Satoshi:25.1.0\/"`, it doesn't seem unique enough because multiple nodes can be running the same version. Won't this cause issues in `find_conn` later?
https://chainquery.com/bitcoin-cli/getnetworkinfo
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1603491415)
Looking at the sample output of `subversion` - `"subversion": "\/Satoshi:25.1.0\/"`, it doesn't seem unique enough because multiple nodes can be running the same version. Won't this cause issues in `find_conn` later?
https://chainquery.com/bitcoin-cli/getnetworkinfo
💬 ajtowns commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510)
I think the 100kB value for transaction relay was adopted as follows:
* 2010-09-13 In 3df62878c3c satoshi added a 100kB (`MAX_BLOCK_SIZE_GEN/5` with `MBS_GEN = MAX_BLOCK_SIZE/2`) limit on new transactions in `CreateTransaction()`
* 2013-02-04 #2273 In gavin gave that constant a name, and made it apply to transaction relay as well
So I don't think there was any deep thought put into that figure. 25kvB sounds fine to me.
Given the suggestion in the OP that it's easier to increase the v
...
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510)
I think the 100kB value for transaction relay was adopted as follows:
* 2010-09-13 In 3df62878c3c satoshi added a 100kB (`MAX_BLOCK_SIZE_GEN/5` with `MBS_GEN = MAX_BLOCK_SIZE/2`) limit on new transactions in `CreateTransaction()`
* 2013-02-04 #2273 In gavin gave that constant a name, and made it apply to transaction relay as well
So I don't think there was any deep thought put into that figure. 25kvB sounds fine to me.
Given the suggestion in the OP that it's easier to increase the v
...
👍 ryanofsky approved a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975#pullrequestreview-2061065588)
Code review ACK e41667b720372dae8438ea86e9819027e62b54e0. Just improvements to comments since last review.
(https://github.com/bitcoin/bitcoin/pull/29975#pullrequestreview-2061065588)
Code review ACK e41667b720372dae8438ea86e9819027e62b54e0. Just improvements to comments since last review.
💬 ryanofsky commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1603529683)
In commit "blockstorage: split up FindBlockPos function" (064859bbad6984a6ec85c744064abdf757807c58)
note: At this point in the PR, GetSerializeSize is already called by the SaveBlockToDisk function calling it, so calling it again here is a bit inefficient. But this is resolved by the next commit moving the the UpdateBlockInfo call
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1603529683)
In commit "blockstorage: split up FindBlockPos function" (064859bbad6984a6ec85c744064abdf757807c58)
note: At this point in the PR, GetSerializeSize is already called by the SaveBlockToDisk function calling it, so calling it again here is a bit inefficient. But this is resolved by the next commit moving the the UpdateBlockInfo call
👍 stickies-v approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2061046449)
ACK 856c8563ca342303a83977146df22768bb6e2c7e
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2061046449)
ACK 856c8563ca342303a83977146df22768bb6e2c7e
💬 stickies-v commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1603522559)
nit: slightly confusing docstring: the `Options` ref is always returned, error is non-empty if options are not valid. Since `Flatten` is internal, this is probably something that mostly should be documented in the `CTxMemPool` ctor anyway though (highlighting that callers should check that `error.empty()`)
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1603522559)
nit: slightly confusing docstring: the `Options` ref is always returned, error is non-empty if options are not valid. Since `Flatten` is internal, this is probably something that mostly should be documented in the `CTxMemPool` ctor anyway though (highlighting that callers should check that `error.empty()`)
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603525767)
Updated to track both successful and successful deletions (on debug log, to prevent unnecessarily spamming the logs)
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603525767)
Updated to track both successful and successful deletions (on debug log, to prevent unnecessarily spamming the logs)
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603383480)
I think this refers to the steps listed at the beginning of the file. Not all of them are covered yet. This PR leaves this right before 2 AFAICT
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603383480)
I think this refers to the steps listed at the beginning of the file. Not all of them are covered yet. This PR leaves this right before 2 AFAICT
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603536079)
Removed
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603536079)
Removed
💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2115495260)
> > FWIW I tried something similar, and got an assertion failure in one of the mempool functional tests (maybe mempool_reorg, and thus due to an invalidateblock call?)
>
> Was it mempool_packages.py maybe? Mine tripped there on `invalidateblock` when I was adding `UpdatedBlockTip` to `InvalidateBlock`, and it was a lock ordering problem. My hierarchy is cs_main -> tx_download_mutex -> mempool.cs.
Ah, yes, I think it was.
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2115495260)
> > FWIW I tried something similar, and got an assertion failure in one of the mempool functional tests (maybe mempool_reorg, and thus due to an invalidateblock call?)
>
> Was it mempool_packages.py maybe? Mine tripped there on `invalidateblock` when I was adding `UpdatedBlockTip` to `InvalidateBlock`, and it was a lock ordering problem. My hierarchy is cs_main -> tx_download_mutex -> mempool.cs.
Ah, yes, I think it was.
💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603548036)
My bad, missed that. Thanks.
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1603548036)
My bad, missed that. Thanks.
⚠️ lctteam opened an issue: "dumpprivkey error"
(https://github.com/bitcoin/bitcoin/issues/30124)
bitcoin-core version: 27.0
I execute: `bitcoin-cli getnewaddress`
and
`bitcoin-cli dumpprivkey "address"`
report an error:
```
error code: -4
error message:
Only legacy wallets are supported by this command
```
I need to export the private key of the address, is there any way to do that?
If I want to use dumpprivkey which version of bitcoin core should I use?
thanks.
(https://github.com/bitcoin/bitcoin/issues/30124)
bitcoin-core version: 27.0
I execute: `bitcoin-cli getnewaddress`
and
`bitcoin-cli dumpprivkey "address"`
report an error:
```
error code: -4
error message:
Only legacy wallets are supported by this command
```
I need to export the private key of the address, is there any way to do that?
If I want to use dumpprivkey which version of bitcoin core should I use?
thanks.
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2115522891)
sorry forgot to link https://github.com/bitcoin/bitcoin/pull/30072 here, added to OP
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2115522891)
sorry forgot to link https://github.com/bitcoin/bitcoin/pull/30072 here, added to OP
🚀 ryanofsky merged a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975)
(https://github.com/bitcoin/bitcoin/pull/29975)
✅ willcl-ark closed an issue: "dumpprivkey error"
(https://github.com/bitcoin/bitcoin/issues/30124)
(https://github.com/bitcoin/bitcoin/issues/30124)
💬 willcl-ark commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30124#issuecomment-2115538080)
You should be able to get the info you need using `bitcoin-cli listdescriptors true`, to include private descriptors.
If that doesn't work for you, you can create a legacy BDB wallet (using a newer version of the software) by following the v26.0 wallet [release notes](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-26.0.md#wallet). Of particular importance is use of the `-deprecatedrpc=create_bdb` option to `bitcoind`, before running `bitcoin-cli createwallet ..
...
(https://github.com/bitcoin/bitcoin/issues/30124#issuecomment-2115538080)
You should be able to get the info you need using `bitcoin-cli listdescriptors true`, to include private descriptors.
If that doesn't work for you, you can create a legacy BDB wallet (using a newer version of the software) by following the v26.0 wallet [release notes](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-26.0.md#wallet). Of particular importance is use of the `-deprecatedrpc=create_bdb` option to `bitcoind`, before running `bitcoin-cli createwallet ..
...
🤔 ryanofsky reviewed a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2061116999)
Updated b58156701ac132f87b8ef8da1c7d22158c804a81 -> 7689dfd6646054a0be8e62ccbf72d5403e28b548 ([`pr/rmutil.17`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.17) -> [`pr/rmutil.18`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.18), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.17..pr/rmutil.18)) moving check-deps script.
---
re: https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055052953
> The includes could be cleaned up a bit, especi
...
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2061116999)
Updated b58156701ac132f87b8ef8da1c7d22158c804a81 -> 7689dfd6646054a0be8e62ccbf72d5403e28b548 ([`pr/rmutil.17`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.17) -> [`pr/rmutil.18`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.18), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.17..pr/rmutil.18)) moving check-deps script.
---
re: https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055052953
> The includes could be cleaned up a bit, especi
...
💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1603561157)
re: https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1599843905
> I think rather than `test/` this should go into `contrib/devtools`, which contains functionally similar scripts like `symbol-check`, or `security-check`.
Good suggestion, it does fit in more with those tools, moved over there.
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1603561157)
re: https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1599843905
> I think rather than `test/` this should go into `contrib/devtools`, which contains functionally similar scripts like `symbol-check`, or `security-check`.
Good suggestion, it does fit in more with those tools, moved over there.
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1603602382)
Removed now, thanks
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1603602382)
Removed now, thanks
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1603602970)
Removed and added a scripted-diff commit to rename `EraseTxNoLock` to `EraseTxInternal`
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1603602970)
Removed and added a scripted-diff commit to rename `EraseTxNoLock` to `EraseTxInternal`
💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1574716741)
While in here, maybe fix the typo
```suggestion
By default the cookie is stored in the data directory, but its location can be
```
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1574716741)
While in here, maybe fix the typo
```suggestion
By default the cookie is stored in the data directory, but its location can be
```