💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249236129)
* Reworded initial sentence to account for fee estimation not being a policy itself.
* Use plain "size" instead of "`vsize`" to avoid loading foot-gun around `getrawtransaction`'s `vsize`.
* Add sentence motivate reasoning behind initial recommendation.
* Bring up the `-bytespersigop` sentence up to section around sigop-adjusted size and split paragraph.
* Use non-breaking space instead of comma as numeric separator for consistency with "4 000 000 units" above.
* Avoid detailing RPCs as the
...
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249236129)
* Reworded initial sentence to account for fee estimation not being a policy itself.
* Use plain "size" instead of "`vsize`" to avoid loading foot-gun around `getrawtransaction`'s `vsize`.
* Add sentence motivate reasoning behind initial recommendation.
* Bring up the `-bytespersigop` sentence up to section around sigop-adjusted size and split paragraph.
* Use non-breaking space instead of comma as numeric separator for consistency with "4 000 000 units" above.
* Avoid detailing RPCs as the
...
💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249228894)
Would be better to change "when" to "where" in the beginning of the line. Sorry for not noticing initially!
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249228894)
Would be better to change "when" to "where" in the beginning of the line. Sorry for not noticing initially!
💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249224212)
From https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3126433719:
> Addressed in [46acee8](https://github.com/bitcoin/bitcoin/pull/32800/commits/46acee890064e69b89f6cc0bc65c625972a4250d)
The newline is added back in the second commit now, it would be better to change the first commit to avoid removing it.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249224212)
From https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3126433719:
> Addressed in [46acee8](https://github.com/bitcoin/bitcoin/pull/32800/commits/46acee890064e69b89f6cc0bc65c625972a4250d)
The newline is added back in the second commit now, it would be better to change the first commit to avoid removing it.
🤔 hodlinator reviewed a pull request: "assumevalid: log every script validation state change"
(https://github.com/bitcoin/bitcoin/pull/32975#pullrequestreview-3081040650)
Concept ACK
Good to communicate to users why their sync progress has slowed down.
Seems like it could be related to the reports in #32832, worth referencing in the PR description?
(https://github.com/bitcoin/bitcoin/pull/32975#pullrequestreview-3081040650)
Concept ACK
Good to communicate to users why their sync progress has slowed down.
Seems like it could be related to the reports in #32832, worth referencing in the PR description?
💬 pablomartin4btc commented on pull request "qt: clear command history when clearing the console":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146493035)
I'm still not very convinced with this change but let's see what others think. Thank you!
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146493035)
I'm still not very convinced with this change but let's see what others think. Thank you!
💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2249245961)
thanks, will fix.
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2249245961)
thanks, will fix.
💬 petertodd commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146508709)
Concept ACK
@benthecarman
> but I believe that the justification for this being the price went up is weak. This suggests also we may just lower it again in the future if the price goes down.
We *should* consider increasing these defaults if the price goes down significantly! E.g. if miners consistently set higher defaults and we're relying transactions that aren't getting mined. Equally, we may want to lower these defaults yet again in the future.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146508709)
Concept ACK
@benthecarman
> but I believe that the justification for this being the price went up is weak. This suggests also we may just lower it again in the future if the price goes down.
We *should* consider increasing these defaults if the price goes down significantly! E.g. if miners consistently set higher defaults and we're relying transactions that aren't getting mined. Equally, we may want to lower these defaults yet again in the future.
💬 kevkevinpal commented on pull request "wallet: remove use of GetWalletDir and weakly_canonical when creating backup_prefix":
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249260236)
thank you! I've updated them in [d1b71e5](https://github.com/bitcoin/bitcoin/pull/33122/commits/d1b71e5846e125f59cd60468f2d76c9721b1dc06)
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249260236)
thank you! I've updated them in [d1b71e5](https://github.com/bitcoin/bitcoin/pull/33122/commits/d1b71e5846e125f59cd60468f2d76c9721b1dc06)
👍 hodlinator approved a pull request: "refactor,test: follow-ups to multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/33039#pullrequestreview-3081045303)
ACK 86e3a0a8cbd30cfee98f5b4acf4ce6d0a75a3ef0
Was worried that removing the random offset parts of the `xor_bytes_reference` test would remove coverage of the misaligned obfuscation. Instead of running code coverage analysis I applied:
```diff
diff --git a/src/util/obfuscation.h b/src/util/obfuscation.h
index e9a2e6093b..8863af1803 100644
--- a/src/util/obfuscation.h
+++ b/src/util/obfuscation.h
@@ -39,7 +39,7 @@ public:
// Obfuscate until KEY_SIZE alignment boundary
...
(https://github.com/bitcoin/bitcoin/pull/33039#pullrequestreview-3081045303)
ACK 86e3a0a8cbd30cfee98f5b4acf4ce6d0a75a3ef0
Was worried that removing the random offset parts of the `xor_bytes_reference` test would remove coverage of the misaligned obfuscation. Instead of running code coverage analysis I applied:
```diff
diff --git a/src/util/obfuscation.h b/src/util/obfuscation.h
index e9a2e6093b..8863af1803 100644
--- a/src/util/obfuscation.h
+++ b/src/util/obfuscation.h
@@ -39,7 +39,7 @@ public:
// Obfuscate until KEY_SIZE alignment boundary
...
💬 hodlinator commented on pull request "refactor,test: follow-ups to multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2249266612)
nit: Commit message for e5b1b7c5577ee36b5bcfb6c02b92da88455411e9 could include a short motivation instead of just the link to the other PR's comment thread:
```diff
+ Makes more sense as we are no longer serializing the obfuscation data into an intermediate "key"-variable.
```
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2249266612)
nit: Commit message for e5b1b7c5577ee36b5bcfb6c02b92da88455411e9 could include a short motivation instead of just the link to the other PR's comment thread:
```diff
+ Makes more sense as we are no longer serializing the obfuscation data into an intermediate "key"-variable.
```
💬 hodlinator commented on pull request "refactor,test: follow-ups to multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2249250632)
nit:
```suggestion
// Aligned obfuscation in 8*KEY_SIZE chunks to nudge compilers toward deploying SIMD
```
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2249250632)
nit:
```suggestion
// Aligned obfuscation in 8*KEY_SIZE chunks to nudge compilers toward deploying SIMD
```
💬 hodlinator commented on pull request "refactor,test: follow-ups to multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2249251032)
nit:
```suggestion
// Aligned obfuscation of remaining KEY_SIZE chunks
```
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2249251032)
nit:
```suggestion
// Aligned obfuscation of remaining KEY_SIZE chunks
```
💬 Sammie05 commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3146551131)
Tested locally. Changes are minimal and improve flexibility for forks. Looks good.
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3146551131)
Tested locally. Changes are minimal and improve flexibility for forks. Looks good.
⚠️ hebasto opened an issue: "OpenBSD, NetBSD: `-reindex` is broken"
(https://github.com/bitcoin/bitcoin/issues/33128)
The `-reindex` option is broken on both the master branch and the 29.x branch (I have not tested older release branches).
Consider the following workflow on OpenBSD 7.7:
```
$ rm -rf /home/hebasto/.bitcoin/regtest
$ ./build/bin/bitcoind -regtest -daemon
Bitcoin Core starting
$ ./build/bin/bitcoin-cli -regtest createwallet $(date +%Y-%m-%d)
$ ./build/bin/bitcoin-cli -regtest -generate 200
$ ./build/bin/bitcoin-cli -regtest getblockcount
200
$ ./build/bin/bitcoin-cli -regtest stop
Bitcoin Core st
...
(https://github.com/bitcoin/bitcoin/issues/33128)
The `-reindex` option is broken on both the master branch and the 29.x branch (I have not tested older release branches).
Consider the following workflow on OpenBSD 7.7:
```
$ rm -rf /home/hebasto/.bitcoin/regtest
$ ./build/bin/bitcoind -regtest -daemon
Bitcoin Core starting
$ ./build/bin/bitcoin-cli -regtest createwallet $(date +%Y-%m-%d)
$ ./build/bin/bitcoin-cli -regtest -generate 200
$ ./build/bin/bitcoin-cli -regtest getblockcount
200
$ ./build/bin/bitcoin-cli -regtest stop
Bitcoin Core st
...
💬 Christewart commented on pull request "test: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify fix":
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3146567971)
@pablomartin4btc @Sammie05 Done in 3543bfd
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3146567971)
@pablomartin4btc @Sammie05 Done in 3543bfd
💬 chrisguida commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146618240)
This seems like it will only lead to more utxoset bloat.
Aren't most of the transactions being confirmed now with <1sat/vB fees just inscriptions and other spam?
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146618240)
This seems like it will only lead to more utxoset bloat.
Aren't most of the transactions being confirmed now with <1sat/vB fees just inscriptions and other spam?
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3146639801)
I'm unable to reproduce the ci failure when I run that job locally -- anyone have an idea?
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3146639801)
I'm unable to reproduce the ci failure when I run that job locally -- anyone have an idea?
💬 caesrcd commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146648378)
@chrisguida
> Aren't most of the transactions being confirmed now with <1sat/vB fees just inscriptions and other spam?
Inscriptions do not rely on wallets to create transactions with <1sat/vB fees. They are typically constructed and broadcasted by custom tools, independent of standard wallet behavior.
The fact that many low‑fee transactions are inscriptions does not imply that regular users are not interested in paying lower fees.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146648378)
@chrisguida
> Aren't most of the transactions being confirmed now with <1sat/vB fees just inscriptions and other spam?
Inscriptions do not rely on wallets to create transactions with <1sat/vB fees. They are typically constructed and broadcasted by custom tools, independent of standard wallet behavior.
The fact that many low‑fee transactions are inscriptions does not imply that regular users are not interested in paying lower fees.
💬 nervana21 commented on pull request "test: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify fix":
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3146658039)
tACK 3543bfd
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3146658039)
tACK 3543bfd
💬 chrisguida commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146667591)
@caesrcd you're missing the point. The point is that inscriptions have proven extremely destructive to the utxoset, and lowering the minimum fee rate makes bloating the utxoset even easier than it was before.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3146667591)
@caesrcd you're missing the point. The point is that inscriptions have proven extremely destructive to the utxoset, and lowering the minimum fee rate makes bloating the utxoset even easier than it was before.