Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ’¬ 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.
πŸ€” furszy reviewed a pull request: "Wallet: Add `max_tx_weight` to transaction funding options (take 2)"
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2129092322)
few small comments
πŸ’¬ furszy commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1646722661)
In d9ec6e7f28f0:

Instead of setting it to 0, could error out early if `max_selection_weight <= 0` (a test for this would be nice).
πŸ’¬ furszy commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1646723399)
Same as above, would be nice to error out early if `max_selection_weight <= 0`. No need to run the selection algorithms under such circumstance.
πŸ’¬ furszy commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1646724513)
nit: could replace [this line](https://github.com/bitcoin/bitcoin/blob/2d21060af831fa8f8f1791b4464961d5f28a88cb/test/functional/p2p_segwit.py#L1265) with this constant.
πŸ’¬ fjahr commented on pull request "p2p: For assumeutxo, download snapshot chain before background chain":
(https://github.com/bitcoin/bitcoin/pull/29519#issuecomment-2179525062)
> yes, but as you mentioned only if all peers were on that other chain.

It may be worth logging something every time we ignore a peer that is on another chain. In case we encounter this (very unlikely) scenario it would help identify the issue faster and it could be an interesting data point alone to know if anyone ever sees this at all.
πŸ’¬ furszy commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1646736262)
Rebase issues in ed00e60c8e8 that are removed on 06caba093a.
⚠️ techy2 opened an issue: "automake script error building 32 bit depends libevent-2.1.12"
(https://github.com/bitcoin/bitcoin/issues/30311)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

building depends for native platform
configure fails to find usable gcc version because of -V -qversion, etc... confusion
this looks like a scripting error

### Expected behaviour

finds current version of gcc on this host of 10.5.0

### Steps to reproduce

on focal32 host
git clone https://github.com/bitcoin/bitcoin
cd bitcoin
git checkout v27.1
cd depends
make


### Relevant log
...
πŸ’¬ ajtowns commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#discussion_r1646884644)
The old text here seems like a better explanation of why you would change this from the new default?
πŸ’¬ maflcko commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2179868020)
```
gcc: fatal error: cannot execute 'cc1': execvp: No such file or directory
```

```
sudo apt-get update
sudo apt-get install --reinstall build-essential
```

Source: https://stackoverflow.com/a/44708372