📝 Sjors converted_to_draft a pull request: "Add waitFeesChanged() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31003)
The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it's time to do so.
I split the implementation into two parts because I'm not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.
The first commit contains a mock implementation very similiar to how longpolling in `getblocktemplate` works. It calls `getTransactionsUpdated` once per second. This returns `true` anytime a transacti
...
(https://github.com/bitcoin/bitcoin/pull/31003)
The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it's time to do so.
I split the implementation into two parts because I'm not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.
The first commit contains a mock implementation very similiar to how longpolling in `getblocktemplate` works. It calls `getTransactionsUpdated` once per second. This returns `true` anytime a transacti
...
💬 Sjors commented on pull request "refactor: mining interface 30955 followups":
(https://github.com/bitcoin/bitcoin/pull/31197#discussion_r1834587201)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31197#discussion_r1834587201)
Will do if I need to retouch.
💬 Sjors commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#issuecomment-2465065423)
Just a comment about how this interacts with the Mining interface changes I'm working on.
This PR adds a new field to CBlockTemplate. The Mining interface contains BlockTemplate:
https://github.com/bitcoin/bitcoin/blob/018e5fcc462caebb25cf5d3eb6a19dc2787466c8/src/interfaces/mining.h#L31-L40
It has a method to return the `CBlockHeader`. I wonder if adding a field to CBlockTemplate would cause a breaking change for clients connecting via IPC. cc @ryanofsky?
In any case it won't matter
...
(https://github.com/bitcoin/bitcoin/pull/30391#issuecomment-2465065423)
Just a comment about how this interacts with the Mining interface changes I'm working on.
This PR adds a new field to CBlockTemplate. The Mining interface contains BlockTemplate:
https://github.com/bitcoin/bitcoin/blob/018e5fcc462caebb25cf5d3eb6a19dc2787466c8/src/interfaces/mining.h#L31-L40
It has a method to return the `CBlockHeader`. I wonder if adding a field to CBlockTemplate would cause a breaking change for clients connecting via IPC. cc @ryanofsky?
In any case it won't matter
...
💬 mzumsande commented on pull request "fuzz: fix bad alloc in connman target":
(https://github.com/bitcoin/bitcoin/pull/31235#discussion_r1834609945)
> Do you remember the bug number? "The reason max_pct exists is not to reveal the entire addrman", but then passing even == 100 is a bug?
GetAddr answers have been capped by percentage at least since addrman was introduced (5fee401fe14aa6459428a26a82f764db70a6a0b9), just that `ADDRMAN_GETADDR_MAX_PCT=23` was hardcoded in the past. It's kind of still that way (`ADDRMAN_GETADDR_MAX_PCT` is used as an arg in net_processing, the rpc code path doesn't use `max_pct`.), so in current master nothing
...
(https://github.com/bitcoin/bitcoin/pull/31235#discussion_r1834609945)
> Do you remember the bug number? "The reason max_pct exists is not to reveal the entire addrman", but then passing even == 100 is a bug?
GetAddr answers have been capped by percentage at least since addrman was introduced (5fee401fe14aa6459428a26a82f764db70a6a0b9), just that `ADDRMAN_GETADDR_MAX_PCT=23` was hardcoded in the past. It's kind of still that way (`ADDRMAN_GETADDR_MAX_PCT` is used as an arg in net_processing, the rpc code path doesn't use `max_pct`.), so in current master nothing
...
📝 fanquake locked a pull request: "doc: Add missing 'blank=true' option in offline-signing-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31236)
### **Issue:**
The text mentions that the `createwallet` command should use the options `disable_private_keys=true, blank=true`, but the provided command only includes `disable_private_keys=true`, missing the `blank=true` option.
---
### **Details:**
**Original Text:**
> This is achieved by using the `createwallet` options: `disable_private_keys=true, blank=true`.
**Original Command:**
```sh
[online]$ ./build/src/bitcoin-cli -signet -named createwallet \
wall
...
(https://github.com/bitcoin/bitcoin/pull/31236)
### **Issue:**
The text mentions that the `createwallet` command should use the options `disable_private_keys=true, blank=true`, but the provided command only includes `disable_private_keys=true`, missing the `blank=true` option.
---
### **Details:**
**Original Text:**
> This is achieved by using the `createwallet` options: `disable_private_keys=true, blank=true`.
**Original Command:**
```sh
[online]$ ./build/src/bitcoin-cli -signet -named createwallet \
wall
...
🤔 jonatack reviewed a pull request: "test: addrman: tried 3 times and never a success so `isTerrible=true`"
(https://github.com/bitcoin/bitcoin/pull/30445#pullrequestreview-2424312395)
ACK d6fabbe2d40b53f6fa35d6b0191f3d73999d6b41
Modulo mentioning and clarifying the time aspect in the test (see comment below) and the PR description.
(https://github.com/bitcoin/bitcoin/pull/30445#pullrequestreview-2424312395)
ACK d6fabbe2d40b53f6fa35d6b0191f3d73999d6b41
Modulo mentioning and clarifying the time aspect in the test (see comment below) and the PR description.
💬 jonatack commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1834706023)
The time setting here seems to be critical and could be commented. It looks like the threshold between success and failure is between `60s` and `61s`. Is that the case for you as well, and could you explain here why?
(https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1834706023)
The time setting here seems to be critical and could be commented. It looks like the threshold between success and failure is between `60s` and `61s`. Is that the case for you as well, and could you explain here why?
💬 achow101 commented on pull request "test: Add combinerawtransaction test to rpc_createmultisig":
(https://github.com/bitcoin/bitcoin/pull/31249#issuecomment-2465262582)
> Forgot to move [ab98e6f](https://github.com/bitcoin/bitcoin/commit/ab98e6fd03970d6b5a593674c84e762a47b90ea6) as well?
Pulled those in too
(https://github.com/bitcoin/bitcoin/pull/31249#issuecomment-2465262582)
> Forgot to move [ab98e6f](https://github.com/bitcoin/bitcoin/commit/ab98e6fd03970d6b5a593674c84e762a47b90ea6) as well?
Pulled those in too
💬 achow101 commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1834730377)
Removed
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1834730377)
Removed
💬 maflcko commented on pull request "test: Add combinerawtransaction test to rpc_createmultisig":
(https://github.com/bitcoin/bitcoin/pull/31249#issuecomment-2465289666)
re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
(https://github.com/bitcoin/bitcoin/pull/31249#issuecomment-2465289666)
re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1834783665)
Indeed, removed.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1834783665)
Indeed, removed.
💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2465442292)
> Yeah, don't worry about the ctest bug. It is unrelated and should be fixed somewhere else. I was mostly wondering why a force push that does not change behavior ([#31000 (comment)](https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)) suddenly makes the CI trip. Why would CI pass before and not after, when `-testdatadir` isn't set. Maybe someone can run the two commits locally on Windows?
Ok, the force-push actually contained a behavior change. It changed the default test
...
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2465442292)
> Yeah, don't worry about the ctest bug. It is unrelated and should be fixed somewhere else. I was mostly wondering why a force push that does not change behavior ([#31000 (comment)](https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)) suddenly makes the CI trip. Why would CI pass before and not after, when `-testdatadir` isn't set. Maybe someone can run the two commits locally on Windows?
Ok, the force-push actually contained a behavior change. It changed the default test
...
📝 furszy opened a pull request: "ci: make ctest stop on failure"
(https://github.com/bitcoin/bitcoin/pull/31257)
Make `ctest` stops when the first failure happens.
Wasting less resources and notifying the developer faster when a failure occurs.
(https://github.com/bitcoin/bitcoin/pull/31257)
Make `ctest` stops when the first failure happens.
Wasting less resources and notifying the developer faster when a failure occurs.
⚠️ 1440000bytes opened an issue: "Add wallet RPC to get new taproot address from a silent payment address"
(https://github.com/bitcoin/bitcoin/issues/31258)
### Please describe the feature you'd like to see added.
RPC that allows generating addresses from a given silent payment address.
### Is your feature related to a problem, if so please describe it.
This RPC could help application developers that use bitcoin core RPC in different projects to generate new addresses from a given silent payment address.
### Describe the solution you'd like
[`getnewaddress`](https://bitcoincore.org/en/doc/28.0.0/rpc/wallet/getnewaddress/) could have a new argum
...
(https://github.com/bitcoin/bitcoin/issues/31258)
### Please describe the feature you'd like to see added.
RPC that allows generating addresses from a given silent payment address.
### Is your feature related to a problem, if so please describe it.
This RPC could help application developers that use bitcoin core RPC in different projects to generate new addresses from a given silent payment address.
### Describe the solution you'd like
[`getnewaddress`](https://bitcoincore.org/en/doc/28.0.0/rpc/wallet/getnewaddress/) could have a new argum
...
💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2465480709)
Updated. Ready to go.
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2465480709)
Updated. Ready to go.
💬 achow101 commented on issue "Add wallet RPC to get new taproot address from a silent payment address":
(https://github.com/bitcoin/bitcoin/issues/31258#issuecomment-2465484175)
Given that computing the actual output script in a transaction paying to a silent payments address requires knowing the exact inputs and having the private keys for those inputs, I don't think this will really be possible in an ergonomic way, and any way to implement this is very footgun-y. The taproot address is entirely dependent on the inputs used so the receiver does not necessarily know that they have been paid if you just send to any taproot address computed from a silent payments address.
...
(https://github.com/bitcoin/bitcoin/issues/31258#issuecomment-2465484175)
Given that computing the actual output script in a transaction paying to a silent payments address requires knowing the exact inputs and having the private keys for those inputs, I don't think this will really be possible in an ergonomic way, and any way to implement this is very footgun-y. The taproot address is entirely dependent on the inputs used so the receiver does not necessarily know that they have been paid if you just send to any taproot address computed from a silent payments address.
...
🤔 BrandonOdiwuor reviewed a pull request: "test: Add combinerawtransaction test to rpc_createmultisig"
(https://github.com/bitcoin/bitcoin/pull/31249#pullrequestreview-2424605718)
Re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
(https://github.com/bitcoin/bitcoin/pull/31249#pullrequestreview-2424605718)
Re-ACK 83fab3212c91d91fc5502f940c901a07772ff747
💬 achow101 commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2465487012)
ACK 47f50c7af5572520fd986b313a63a44a76d3c859
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2465487012)
ACK 47f50c7af5572520fd986b313a63a44a76d3c859
✅ achow101 closed an issue: "man pages have a useless descriptions"
(https://github.com/bitcoin/bitcoin/issues/29552)
(https://github.com/bitcoin/bitcoin/issues/29552)
🚀 achow101 merged a pull request: "Update manpage descriptions"
(https://github.com/bitcoin/bitcoin/pull/29686)
(https://github.com/bitcoin/bitcoin/pull/29686)