π am-sq's pull request is ready for review: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302)
(https://github.com/bitcoin/bitcoin/pull/30302)
π¬ am-sq commented on issue "Improve description of the `filename` parameter of `loadwallet` RPC":
(https://github.com/bitcoin/bitcoin/issues/30269#issuecomment-2179363155)
Wanted to explicitly mention I have opened https://github.com/bitcoin/bitcoin/pull/30302 to resolve this issue. Thanks in advance for guidance and review.
(https://github.com/bitcoin/bitcoin/issues/30269#issuecomment-2179363155)
Wanted to explicitly mention I have opened https://github.com/bitcoin/bitcoin/pull/30302 to resolve this issue. Thanks in advance for guidance and review.
π¬ fjahr commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#issuecomment-2179369598)
Needed to rebase because of a silent merge conflict.
(https://github.com/bitcoin/bitcoin/pull/30267#issuecomment-2179369598)
Needed to rebase because of a silent merge conflict.
π€ hodlinator requested changes to a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2128956201)
nACK dc4cbe209508b2f95a748137b200056451405c9e
Commits need squashing together.
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2128956201)
nACK dc4cbe209508b2f95a748137b200056451405c9e
Commits need squashing together.
π¬ hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646635390)
Tried `createwallet` to observe that the current version creates a directory with the given name, containing `wallet.dat` (and `wallet.dat-journal` while loaded).
Suggestion: put the normal/current/simplest operation first, and also add some descriptions to command examples:
```suggestion
"\nLoad regular wallet with files under wallets/foo/:\n"
+ HelpExampleCli("loadwallet", "\"foo\"") +
"\nLoad wallet using absolute path:\n"
...
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646635390)
Tried `createwallet` to observe that the current version creates a directory with the given name, containing `wallet.dat` (and `wallet.dat-journal` while loaded).
Suggestion: put the normal/current/simplest operation first, and also add some descriptions to command examples:
```suggestion
"\nLoad regular wallet with files under wallets/foo/:\n"
+ HelpExampleCli("loadwallet", "\"foo\"") +
"\nLoad wallet using absolute path:\n"
...
π¬ hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646638086)
`/Users/joe/specialWallets` should be changed to `/Users/joe/specialWallet/` here and in the RPC example since we only load one wallet at a time and to make clear that it is a directory.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646638086)
`/Users/joe/specialWallets` should be changed to `/Users/joe/specialWallet/` here and in the RPC example since we only load one wallet at a time and to make clear that it is a directory.
π¬ Prabhat1308 commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1646641952)
I got some data for about 4100+ transactions from a source (cant be sure of the credibility) for bitcoin transactions in a mempool and i ran a script to count the avg. parents which came to approx. 1.6 . Another way I thought of this is I got the avg. number of [inputs per transaction](https://transactionfee.info/charts/inputs-per-transaction/) . For 2023-2024 the average is somewhat between 2-3 . If we make a tree like dependency structure for transaction considering the transactions which had
...
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1646641952)
I got some data for about 4100+ transactions from a source (cant be sure of the credibility) for bitcoin transactions in a mempool and i ran a script to count the avg. parents which came to approx. 1.6 . Another way I thought of this is I got the avg. number of [inputs per transaction](https://transactionfee.info/charts/inputs-per-transaction/) . For 2023-2024 the average is somewhat between 2-3 . If we make a tree like dependency structure for transaction considering the transactions which had
...
π¬ am-sq commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646668328)
Thanks for the suggestion - fixed.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646668328)
Thanks for the suggestion - fixed.
π¬ am-sq commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646668494)
Great suggestion- added!
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646668494)
Great suggestion- added!
π¬ am-sq commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646669081)
Added additional examples
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646669081)
Added additional examples
π tdb3 approved a pull request: "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core""
(https://github.com/bitcoin/bitcoin/pull/30308#pullrequestreview-2129017727)
ACK 197b5404b0f4a1d6e989000845b45c8bd24e0bc6
grepped for other instances of `Bitcoin Core` in test/functional/ and didn't see any beyond what @kevkevinpal identified.
(https://github.com/bitcoin/bitcoin/pull/30308#pullrequestreview-2129017727)
ACK 197b5404b0f4a1d6e989000845b45c8bd24e0bc6
grepped for other instances of `Bitcoin Core` in test/functional/ and didn't see any beyond what @kevkevinpal identified.
π hodlinator approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2129019940)
ACK c3bd87370a274adab9ba2d07c87508d3eded6be7
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2129019940)
ACK c3bd87370a274adab9ba2d07c87508d3eded6be7
π¬ hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646682005)
```suggestion
+ "\nLoad regular wallet with files under wallets/foo/specialWallet/:\n"
```
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646682005)
```suggestion
+ "\nLoad regular wallet with files under wallets/foo/specialWallet/:\n"
```
π¬ ismaelsadeeq commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646688833)
nit: the v3 mentions in debug strings should be explicitly stated as version=3 for clarity.
here and other places
```suggestion
return std::make_pair(strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
```
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646688833)
nit: the v3 mentions in debug strings should be explicitly stated as version=3 for clarity.
here and other places
```suggestion
return std::make_pair(strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
```
π¬ ismaelsadeeq commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646699035)
Add `CheckMempoolV3Invariants` also?
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646699035)
Add `CheckMempoolV3Invariants` also?
π¬ ismaelsadeeq commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646705728)
Maybe update `test/functional/wallet_create_tx.py` test.
see https://github.com/bitcoin/bitcoin/commit/d3fa531e67c0af77f52e754ad47a8396f2cab4c4, if it makes sense feel free to cherry-pick
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646705728)
Maybe update `test/functional/wallet_create_tx.py` test.
see https://github.com/bitcoin/bitcoin/commit/d3fa531e67c0af77f52e754ad47a8396f2cab4c4, if it makes sense feel free to cherry-pick
π¬ ismaelsadeeq commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646698050)
In f5b26e5b9be41cdab6e75aa0a3fbbb765a96ed06
This left out v3 mentions in the unit tests, maybe just leave out just the variable names as you stated from PR description.
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646698050)
In f5b26e5b9be41cdab6e75aa0a3fbbb765a96ed06
This left out v3 mentions in the unit tests, maybe just leave out just the variable names as you stated from PR description.
π¬ ismaelsadeeq commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646693859)
In f5b26e5b9be41cdab6e75aa0a3fbbb765a96ed06
Here are a couple of places that were not replaced
```diff
diff --git a/src/policy/truc_policy.h b/src/policy/truc_policy.h
index d1e65806cc0..37a445bcc14 100644
--- a/src/policy/truc_policy.h
+++ b/src/policy/truc_policy.h
@@ -23,18 +23,18 @@ static constexpr decltype(CTransaction::version) TRUC_VERSION{3};
// of 2 and ancestor set size of 2.
/** Maximum number of transactions including an unconfirmed tx and its descendants. */
static con
...
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1646693859)
In f5b26e5b9be41cdab6e75aa0a3fbbb765a96ed06
Here are a couple of places that were not replaced
```diff
diff --git a/src/policy/truc_policy.h b/src/policy/truc_policy.h
index d1e65806cc0..37a445bcc14 100644
--- a/src/policy/truc_policy.h
+++ b/src/policy/truc_policy.h
@@ -23,18 +23,18 @@ static constexpr decltype(CTransaction::version) TRUC_VERSION{3};
// of 2 and ancestor set size of 2.
/** Maximum number of transactions including an unconfirmed tx and its descendants. */
static con
...
π¬ fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2179487330)
> Do blocks mined with the exception count towards the total work according to difficulty-1 or the actual difficulty? If itβs the latter, in testnet3 you could just invalidate the last block in the previous difficulty period with a difficulty-1 block. If itβs the former, I agree, the absence of this attack may indicate that Iβm worrying too much.
(Side-note: I first read difficulty-1 as "difficulty minus 1" and got a bit confused but I think you mean the same as "difficulty=1")
I needed to
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2179487330)
> Do blocks mined with the exception count towards the total work according to difficulty-1 or the actual difficulty? If itβs the latter, in testnet3 you could just invalidate the last block in the previous difficulty period with a difficulty-1 block. If itβs the former, I agree, the absence of this attack may indicate that Iβm worrying too much.
(Side-note: I first read difficulty-1 as "difficulty minus 1" and got a bit confused but I think you mean the same as "difficulty=1")
I needed to
...
π hodlinator approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2129076496)
ACK ee15876ab33ad2487d7f1126ed653f9ae43d0514
Compiled and ran `bitcoin-cli --regtest loadwallet` without parameters. I like how you ended up alternating CLI/RPC examples.
Tested `loadwallet "foo"` just to make sure nothing funny happens with extra quotes.
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2129076496)
ACK ee15876ab33ad2487d7f1126ed653f9ae43d0514
Compiled and ran `bitcoin-cli --regtest loadwallet` without parameters. I like how you ended up alternating CLI/RPC examples.
Tested `loadwallet "foo"` just to make sure nothing funny happens with extra quotes.