👍 TheCharlatan approved a pull request: "refactor: Make node_id a const& in RemoveBlockRequest"
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2436015897)
ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2436015897)
ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
👋 Sjors's pull request is ready for review: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)
(https://github.com/bitcoin/bitcoin/pull/31283)
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476309704)
I added test coverage for `waitNext()`.
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476309704)
I added test coverage for `waitNext()`.
💬 TheCharlatan commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476337636)
In commit f9200f6c65b5a97a4842cc1ec34e688ec13bffcd
"Use createNewBlock via the interface instead of calling Chainman's CreateNewBlock."
Do you mean "Use createNewBlock via the interface instead of calling CreateNewBlock through the BlockAssembler directly"? There is no `CreateNewBlock` in the chainman.
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476337636)
In commit f9200f6c65b5a97a4842cc1ec34e688ec13bffcd
"Use createNewBlock via the interface instead of calling Chainman's CreateNewBlock."
Do you mean "Use createNewBlock via the interface instead of calling CreateNewBlock through the BlockAssembler directly"? There is no `CreateNewBlock` in the chainman.
💬 laanwj commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2476350501)
oh neat ! i've been working on a similar export functionality to get the RPCHelpMan data into json, but he beat me to it.
This isn't the format i was targeting, and i still think it's better to avoid introducing a bitcoin-core specific description format, if we can, but exporting it in the first place an important step.
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2476350501)
oh neat ! i've been working on a similar export functionality to get the RPCHelpMan data into json, but he beat me to it.
This isn't the format i was targeting, and i still think it's better to avoid introducing a bitcoin-core specific description format, if we can, but exporting it in the first place an important step.
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842225267)
I dropped this line.
Additionally this PR does not make use of `last_mempool_update`, since it's not very useful in a typical production setting where something in the mempool updates all the time.
I think the race condition mentioned there is not an issue here because we don't pass in the current fees via argument, but instead derive them from the original template.
I did forget to update `now` at the end of the loop, which I fixed.
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842225267)
I dropped this line.
Additionally this PR does not make use of `last_mempool_update`, since it's not very useful in a typical production setting where something in the mempool updates all the time.
I think the race condition mentioned there is not an issue here because we don't pass in the current fees via argument, but instead derive them from the original template.
I did forget to update `now` at the end of the loop, which I fixed.
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476365392)
Made some adjustments based on https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842225267
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476365392)
Made some adjustments based on https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842225267
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476371060)
@TheCharlatan I adjusted the commit message
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476371060)
@TheCharlatan I adjusted the commit message
✅ Sjors closed a pull request: "Add waitFeesChanged() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31003)
(https://github.com/bitcoin/bitcoin/pull/31003)
💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2476375818)
Actually I'm convinced the `waitNext()` approach is better.
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2476375818)
Actually I'm convinced the `waitNext()` approach is better.
💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476385674)
> https://github.com/TheCharlatan/bitcoin/commit/1a6724af305122973efcc8a329e98f41077314a2
Pulled that in. Added another commit for sqlite.
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476385674)
> https://github.com/TheCharlatan/bitcoin/commit/1a6724af305122973efcc8a329e98f41077314a2
Pulled that in. Added another commit for sqlite.
💬 hodlinator commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2476390623)
> Do you have a particular test case that triggered there?
Worked through several iterations of the `shared_ptr`-removal over a couple of days, at some point I had:
```C++
ret.emplace_back(std::make_unique<MiniscriptDescriptor>(std::move(pubs), std::move(*node)));
```
'cause I wasn't paying attention to the statement being inside a loop. The second descriptor created in the loop ended up with already-moved-from values, which resulted in an this test failing:
https://github.com/bitcoin/
...
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2476390623)
> Do you have a particular test case that triggered there?
Worked through several iterations of the `shared_ptr`-removal over a couple of days, at some point I had:
```C++
ret.emplace_back(std::make_unique<MiniscriptDescriptor>(std::move(pubs), std::move(*node)));
```
'cause I wasn't paying attention to the statement being inside a loop. The second descriptor created in the loop ended up with already-moved-from values, which resulted in an this test failing:
https://github.com/bitcoin/
...
💬 josibake commented on pull request "wallet, desc spkm: Return SigningProvider only if we have the privkey":
(https://github.com/bitcoin/bitcoin/pull/31242#issuecomment-2476403763)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31242#issuecomment-2476403763)
Concept ACK
📝 Sjors opened a pull request: "Add destroy to BlockTemplate schema"
(https://github.com/bitcoin/bitcoin/pull/31288)
This ensures that if a client no longer needs a block template, the node can clear its memory as soon as possible.
A block template may hold on to transactions that are no longer in the mempool, so this can be significant.
(https://github.com/bitcoin/bitcoin/pull/31288)
This ensures that if a client no longer needs a block template, the node can clear its memory as soon as possible.
A block template may hold on to transactions that are no longer in the mempool, so this can be significant.
✅ hodlinator closed a pull request: "test: Fix RANDOM_CTX_SEED use with parallel tests"
(https://github.com/bitcoin/bitcoin/pull/30737)
(https://github.com/bitcoin/bitcoin/pull/30737)
💬 hodlinator commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2476412113)
Closing since the main issue was fixed in a different way by the now merged #31000.
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2476412113)
Closing since the main issue was fixed in a different way by the now merged #31000.
💬 Sjors commented on pull request "Add destroy to BlockTemplate schema":
(https://github.com/bitcoin/bitcoin/pull/31288#issuecomment-2476412321)
@ryanofsky can you further clarify what the difference in behavior is with an without a `destroy` method?
(https://github.com/bitcoin/bitcoin/pull/31288#issuecomment-2476412321)
@ryanofsky can you further clarify what the difference in behavior is with an without a `destroy` method?
👍 theStack approved a pull request: "wallet, desc spkm: Return SigningProvider only if we have the privkey"
(https://github.com/bitcoin/bitcoin/pull/31242#pullrequestreview-2436154466)
Concept and code-review ACK 493656763f73e5ef1cfb979a513c12983dca99dd
Convinced myself that this works as intended by writing a simple unit test: https://github.com/theStack/bitcoin/tree/pr31242_add_test
(could be added here or in a follow-up, though having to change the visibility of `GetSigningProvider` from private to public only for the sake of testing is a bit ugly...)
(https://github.com/bitcoin/bitcoin/pull/31242#pullrequestreview-2436154466)
Concept and code-review ACK 493656763f73e5ef1cfb979a513c12983dca99dd
Convinced myself that this works as intended by writing a simple unit test: https://github.com/theStack/bitcoin/tree/pr31242_add_test
(could be added here or in a follow-up, though having to change the visibility of `GetSigningProvider` from private to public only for the sake of testing is a bit ugly...)
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1842272710)
we do in the following lines
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1842272710)
we do in the following lines
👍 TheCharlatan approved a pull request: "guix: remove `util-linux`"
(https://github.com/bitcoin/bitcoin/pull/31285#pullrequestreview-2436171166)
ACK 4d668549825cc2f999b349e8fcd8cc9434c409c3
Guix build:
```
b681575760a0d19445a17d0d54f50a65d705923caee3a7cd7502e6611950dc11 guix-build-4d668549825c/output/aarch64-linux-gnu/SHA256SUMS.part
7af1420fedd8b4d10df650e92840287106dd74c289b6ef6fd2fd039387824647 guix-build-4d668549825c/output/aarch64-linux-gnu/bitcoin-4d668549825c-aarch64-linux-gnu-debug.tar.gz
bd6520732b5c7805002a3227fe2d16f56a77453d913f5204e5be869aaf4a9534 guix-build-4d668549825c/output/aarch64-linux-gnu/bitcoin-4d668549825
...
(https://github.com/bitcoin/bitcoin/pull/31285#pullrequestreview-2436171166)
ACK 4d668549825cc2f999b349e8fcd8cc9434c409c3
Guix build:
```
b681575760a0d19445a17d0d54f50a65d705923caee3a7cd7502e6611950dc11 guix-build-4d668549825c/output/aarch64-linux-gnu/SHA256SUMS.part
7af1420fedd8b4d10df650e92840287106dd74c289b6ef6fd2fd039387824647 guix-build-4d668549825c/output/aarch64-linux-gnu/bitcoin-4d668549825c-aarch64-linux-gnu-debug.tar.gz
bd6520732b5c7805002a3227fe2d16f56a77453d913f5204e5be869aaf4a9534 guix-build-4d668549825c/output/aarch64-linux-gnu/bitcoin-4d668549825
...