💬 darosior commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078906634)
> If we enter a 90% drawdown bear market, we aren't going to revisit this value and raise it, are we?
Plus, we wouldn't be able to at all for software that is already released. Better have a conservative value that is resistant to an asset price decrease than an optimistic value that tries to captivate very low fees in times of high asset prices and low usage.
I am not against lowering the minrelay value, especially if it is shown that there is sustained usage of sub-1sat/vb transactions t
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078906634)
> If we enter a 90% drawdown bear market, we aren't going to revisit this value and raise it, are we?
Plus, we wouldn't be able to at all for software that is already released. Better have a conservative value that is resistant to an asset price decrease than an optimistic value that tries to captivate very low fees in times of high asset prices and low usage.
I am not against lowering the minrelay value, especially if it is shown that there is sustained usage of sub-1sat/vb transactions t
...
💬 sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210633065)
Correct, I did not want to loop twice over the block's transactions to compute the actual number of spent inputs.
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210633065)
Correct, I did not want to loop twice over the block's transactions to compute the actual number of spent inputs.
💬 sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210635923)
How can I find the tx without the block's offset ?
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210635923)
How can I find the tx without the block's offset ?
💬 l0rinc commented on pull request "[IBD] log start of script validation past `assumevalid` block":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2210648124)
Many users are surprised by the slowdowns that suddenly happen during IBD, this should clarify that. If this doesn't work for some corner cases, I think that should be fine, it's just meant to help explain the shift in IBD performance.
I can change it to include the case when script verification starts initially, but I explicitly meant this log to signal when the effect of assumevalid gets turned off, i.e. to explain "grinding to a halt" that many users were reporting.
> See also https://g
...
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2210648124)
Many users are surprised by the slowdowns that suddenly happen during IBD, this should clarify that. If this doesn't work for some corner cases, I think that should be fine, it's just meant to help explain the shift in IBD performance.
I can change it to include the case when script verification starts initially, but I explicitly meant this log to signal when the effect of assumevalid gets turned off, i.e. to explain "grinding to a halt" that many users were reporting.
> See also https://g
...
💬 Rob1Ham commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078947536)
A naive question I have is how does this relate to another policy change like mempoolfullrbf? We currently have 20% of hashrate now mining sub 1 sat/vbyte ([source](https://x.com/mononautical/status/1944942585384198259)) and some mining pools are leaving revenue on the table due to not having the right mempool policy ([source](https://x.com/mononautical/status/1945328731821892043)), couldn't the argument be made that some mining pools are being advantaged by using the default values?
A fair c
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078947536)
A naive question I have is how does this relate to another policy change like mempoolfullrbf? We currently have 20% of hashrate now mining sub 1 sat/vbyte ([source](https://x.com/mononautical/status/1944942585384198259)) and some mining pools are leaving revenue on the table due to not having the right mempool policy ([source](https://x.com/mononautical/status/1945328731821892043)), couldn't the argument be made that some mining pools are being advantaged by using the default values?
A fair c
...
🤔 furszy reviewed a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3025398994)
It seems we're already thinking about upgrades in #32895 ?.
If that's the case, I'd rather keep the RPC. The maintenance cost should be minimal.
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3025398994)
It seems we're already thinking about upgrades in #32895 ?.
If that's the case, I'd rather keep the RPC. The maintenance cost should be minimal.
🤔 stickies-v reviewed a pull request: "Adds transaction propagation information to mempool transactions"
(https://github.com/bitcoin/bitcoin/pull/32986#pullrequestreview-3025461492)
I'm generally not in favour of making bespoke changes to production code or extending the public interface just to facilitate tests, monitoring, ... if there exists a workaround, especially for temporary projects. In this case, after speaking with @sr-gi offline a bit more, it seems it seems like tracepoints, logging, small patchsets, ... are feasible alternatives, so I'm Concept NACK on this one. (but I am open to making minimal changes to ensure the necessary erlay simulations can be ran)
(https://github.com/bitcoin/bitcoin/pull/32986#pullrequestreview-3025461492)
I'm generally not in favour of making bespoke changes to production code or extending the public interface just to facilitate tests, monitoring, ... if there exists a workaround, especially for temporary projects. In this case, after speaking with @sr-gi offline a bit more, it seems it seems like tracepoints, logging, small patchsets, ... are feasible alternatives, so I'm Concept NACK on this one. (but I am open to making minimal changes to ensure the necessary erlay simulations can be ran)
🤔 stickies-v reviewed a pull request: "doc: clarify GetAddresses documentation"
(https://github.com/bitcoin/bitcoin/pull/32994#pullrequestreview-3025490230)
Concept ACK. I would strongly prefer to go further and rename the trusted function to `GetAddressesUncached`. Overloads that behave vastly differently and can lead to dangerous behaviour is a big red flag for me.
(https://github.com/bitcoin/bitcoin/pull/32994#pullrequestreview-3025490230)
Concept ACK. I would strongly prefer to go further and rename the trusted function to `GetAddressesUncached`. Overloads that behave vastly differently and can lead to dangerous behaviour is a big red flag for me.
🤔 theuni reviewed a pull request: "depends: fix libevent `_WIN32_WINNT` usage"
(https://github.com/bitcoin/bitcoin/pull/32837#pullrequestreview-3025518171)
Concept ACK and vague post-merge utACK.
I don't understand all of the upstream changes, but it makes sense to take that patch if it fixes our issue.
(https://github.com/bitcoin/bitcoin/pull/32837#pullrequestreview-3025518171)
Concept ACK and vague post-merge utACK.
I don't understand all of the upstream changes, but it makes sense to take that patch if it fixes our issue.
💬 theuni commented on pull request "depends: fix libevent `_WIN32_WINNT` usage":
(https://github.com/bitcoin/bitcoin/pull/32837#discussion_r2210729180)
Why was this undef'd before and not now?
(https://github.com/bitcoin/bitcoin/pull/32837#discussion_r2210729180)
Why was this undef'd before and not now?
💬 fanquake commented on pull request "depends: fix libevent `_WIN32_WINNT` usage":
(https://github.com/bitcoin/bitcoin/pull/32837#discussion_r2210782699)
The upstream change just removed all the undefining: https://github.com/libevent/libevent/pull/1498.
(https://github.com/bitcoin/bitcoin/pull/32837#discussion_r2210782699)
The upstream change just removed all the undefining: https://github.com/libevent/libevent/pull/1498.
💬 fanquake commented on pull request "fuzz: wallet: add target for tx scanning":
(https://github.com/bitcoin/bitcoin/pull/32993#issuecomment-3079192425)
https://cirrus-ci.com/task/4897452429410304?logs=ci#L6267:
```bash
[10:48:30.374] Run wallet_scan with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/fuzz', '-max_total_time=60']INFO: Running with entropic power schedule (0xFF, 100).
[10:48:30.374] INFO: Seed: 1927962085
[10:48:30.374] INFO: Loaded 1 modules (634026 inline 8-bit counters): 634026 [0x5573df666fb8, 0x5573df701c62),
[10:48:30.374] INFO: Loaded 1 PC tables (634026 PCs): 634026 [0x5573df701c68,0x5573e00ae7
...
(https://github.com/bitcoin/bitcoin/pull/32993#issuecomment-3079192425)
https://cirrus-ci.com/task/4897452429410304?logs=ci#L6267:
```bash
[10:48:30.374] Run wallet_scan with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/fuzz', '-max_total_time=60']INFO: Running with entropic power schedule (0xFF, 100).
[10:48:30.374] INFO: Seed: 1927962085
[10:48:30.374] INFO: Loaded 1 modules (634026 inline 8-bit counters): 634026 [0x5573df666fb8, 0x5573df701c62),
[10:48:30.374] INFO: Loaded 1 PC tables (634026 PCs): 634026 [0x5573df701c68,0x5573e00ae7
...
📝 brunoerg converted_to_draft a pull request: "fuzz: wallet: add target for tx scanning"
(https://github.com/bitcoin/bitcoin/pull/32993)
Tracking issue (#29901)
This PR adds a fuzz target for wallet tx scanning (`ScanForWalletTransactions`). Unfortunately, it's not a regression test for the issue #31474 since it would require a more complex scenario/async operations.
(https://github.com/bitcoin/bitcoin/pull/32993)
Tracking issue (#29901)
This PR adds a fuzz target for wallet tx scanning (`ScanForWalletTransactions`). Unfortunately, it's not a regression test for the issue #31474 since it would require a more complex scenario/async operations.
💬 murchandamus commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079233470)
> > @BitcoinMechanic:
> > > > The most persuasive argument for this would be that miners have diverged from defaults
> > >
> > > You continue to confuse cause and effect. Miners mine what gets relayed, not the reverse.
> > > Default relay policy for Core essentially controls what ends up in the blockchain and what doesn't. It's pointless to keep pretending otherwise.
> >
> > Neither Bitcoin Core, nor Knots, not even [LibreRelay nodes](https://github.com/petertodd/bitcoin/blob/af41065cb2a
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079233470)
> > @BitcoinMechanic:
> > > > The most persuasive argument for this would be that miners have diverged from defaults
> > >
> > > You continue to confuse cause and effect. Miners mine what gets relayed, not the reverse.
> > > Default relay policy for Core essentially controls what ends up in the blockchain and what doesn't. It's pointless to keep pretending otherwise.
> >
> > Neither Bitcoin Core, nor Knots, not even [LibreRelay nodes](https://github.com/petertodd/bitcoin/blob/af41065cb2a
...
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2210830294)
this fixed the error, thanks!
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2210830294)
this fixed the error, thanks!
⚠️ fanquake opened an issue: "fuzz: AddressSanitizer: odr-violation typeinfo name for CCoinsViewBacked"
(https://github.com/bitcoin/bitcoin/issues/32995)
```
# clang++ --version
clang version 20.1.8 (Fedora 20.1.8-1.fc43)
Target: aarch64-redhat-linux-gnu
make -C depends/ NO_QT=1 NO_ZMQ=1 NO_USDT=1 NO_WALLET=1 AR=llvm-ar NM=llvm-nm RANLIB=llvm-ranlib STRIP=llvm-strip
cmake -B build --toolchain /root/bitcoin/depends/aarch64-unknown-linux-gnu/toolchain.cmake -DBUILD_FOR_FUZZING=ON -DSANITIZERS=address -DAPPEND_CFLAGS="-flto=full" -DAPPEND_CXXFLAGS="-flto=full" -DAPPEND_LDFLAGS="-flto=full"
cmake --build build
```
```
./build/bin/fuzz
==============
...
(https://github.com/bitcoin/bitcoin/issues/32995)
```
# clang++ --version
clang version 20.1.8 (Fedora 20.1.8-1.fc43)
Target: aarch64-redhat-linux-gnu
make -C depends/ NO_QT=1 NO_ZMQ=1 NO_USDT=1 NO_WALLET=1 AR=llvm-ar NM=llvm-nm RANLIB=llvm-ranlib STRIP=llvm-strip
cmake -B build --toolchain /root/bitcoin/depends/aarch64-unknown-linux-gnu/toolchain.cmake -DBUILD_FOR_FUZZING=ON -DSANITIZERS=address -DAPPEND_CFLAGS="-flto=full" -DAPPEND_CXXFLAGS="-flto=full" -DAPPEND_LDFLAGS="-flto=full"
cmake --build build
```
```
./build/bin/fuzz
==============
...
💬 pablomartin4btc commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3079267669)
> migrated wallets will take over the legacy bdb version, which I am not sure is meaningful or useful. The upgradewallet RPC can be used to bump it.
As discussed offline with @achow101 a couple of days ago, my understanding was that both version field and ofc the `upgradewallet` RPC were exclusive to legacy wallets. I think the new migrated wallet has the latest version of features (`walletInstance->SetMinVersion(FEATURE_LATEST)`) as descriptors support everything, so they don't need to be bu
...
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3079267669)
> migrated wallets will take over the legacy bdb version, which I am not sure is meaningful or useful. The upgradewallet RPC can be used to bump it.
As discussed offline with @achow101 a couple of days ago, my understanding was that both version field and ofc the `upgradewallet` RPC were exclusive to legacy wallets. I think the new migrated wallet has the latest version of features (`walletInstance->SetMinVersion(FEATURE_LATEST)`) as descriptors support everything, so they don't need to be bu
...
💬 BitcoinMechanic commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079331769)
>your assessment that "default relay policy for Core essentially controls what ends up in the blockchain and what doesn't" is obviously falsified.
It obviously remains true, you're just cherry picking. Again, the vast, overwhelming, super-super majority of what you see in the chain is what nodes will typically relay due to whatever Core's defaults are.
This is not an "overly general" statement, it's a fact that - I'm sorry to have to point out - is being ignored because that is convenient
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079331769)
>your assessment that "default relay policy for Core essentially controls what ends up in the blockchain and what doesn't" is obviously falsified.
It obviously remains true, you're just cherry picking. Again, the vast, overwhelming, super-super majority of what you see in the chain is what nodes will typically relay due to whatever Core's defaults are.
This is not an "overly general" statement, it's a fact that - I'm sorry to have to point out - is being ignored because that is convenient
...
💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210851772)
nit in 7b93fa394acaee7a715fae9a2b968d33c5adc174: Is the `min` needed? `misalign` is less than KEY_SIZE, and `target.size() > KEY_SIZE`.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210851772)
nit in 7b93fa394acaee7a715fae9a2b968d33c5adc174: Is the `min` needed? `misalign` is less than KEY_SIZE, and `target.size() > KEY_SIZE`.
💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210500694)
nit in ddba3f5866868a6a27cd945508091439c6a82416: I think `XorWord` already explains the processing of multiple bytes at once, so this comment could be removed.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210500694)
nit in ddba3f5866868a6a27cd945508091439c6a82416: I think `XorWord` already explains the processing of multiple bytes at once, so this comment could be removed.