Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ‘‹ am-sq's pull request is ready for review: "doc: clarify loadwallet path loading for wallets"
(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.
πŸ’¬ 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.
πŸ€” 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.
πŸ’¬ 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"

...
πŸ’¬ 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.
πŸ’¬ 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
...
πŸ’¬ 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.
πŸ’¬ 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!
πŸ’¬ 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
πŸ‘ 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.
πŸ‘ hodlinator approved a pull request: "doc: clarify loadwallet path loading for wallets"
(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"
```
πŸ’¬ 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",
```
πŸ’¬ 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?
πŸ’¬ 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
πŸ’¬ 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.
πŸ’¬ 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
...
πŸ’¬ 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
...
πŸ‘ 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.