💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409152)
fixed
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409152)
fixed
💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2417271469)
To address the last point (will respond to the rest later):
> It's also my understanding the the IPC logic in Bitcoin Core is not intended as a public API, and thus users of it are expected to exclusively be in Bitcoin Core, I suppose that is changing here?
In general IPC interfaces are not intended to be public, but the mining interface is an exception. @josibake is working on a proof-of-concept Rust demo that uses it, which should be fairly easy to use in SRI.
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2417271469)
To address the last point (will respond to the rest later):
> It's also my understanding the the IPC logic in Bitcoin Core is not intended as a public API, and thus users of it are expected to exclusively be in Bitcoin Core, I suppose that is changing here?
In general IPC interfaces are not intended to be public, but the mining interface is an exception. @josibake is working on a proof-of-concept Rust demo that uses it, which should be fairly easy to use in SRI.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409321)
Removed the `bypass_limits` along with unification of CheckEphemeralSpends.
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409321)
Removed the `bypass_limits` along with unification of CheckEphemeralSpends.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409405)
done, put the new version in both AcceptMultipleTransactions and AcceptSingleTransaciton
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409405)
done, put the new version in both AcceptMultipleTransactions and AcceptSingleTransaciton
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409501)
changed assert to `Assume` as requested, let it stumble along
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1803409501)
changed assert to `Assume` as requested, let it stumble along
💬 stickies-v commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2417294412)
> it's easier to set up for users, since it doesn't require any form of authentication
It seems `electrs` already uses the authentication cookie by default (as per their docs), so for most setups, this shouldn't be a meaningful difference?
> it's safer...
It seems this new endpoint is aimed at less savvy users or users that don't have an overly complex setup and e.g. just want to run an electrum server for personal use. In that scenario, I'm not sure the footguns of e.g forgetting to wh
...
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2417294412)
> it's easier to set up for users, since it doesn't require any form of authentication
It seems `electrs` already uses the authentication cookie by default (as per their docs), so for most setups, this shouldn't be a meaningful difference?
> it's safer...
It seems this new endpoint is aimed at less savvy users or users that don't have an overly complex setup and e.g. just want to run an electrum server for personal use. In that scenario, I'm not sure the footguns of e.g forgetting to wh
...
👍 jarolrod approved a pull request: "doc: remove dependency install instructions from win docs"
(https://github.com/bitcoin/bitcoin/pull/31100#pullrequestreview-2373009083)
ACK 184f12c1542f6c53eb2bd9dfb08dfdd490e38846
(https://github.com/bitcoin/bitcoin/pull/31100#pullrequestreview-2373009083)
ACK 184f12c1542f6c53eb2bd9dfb08dfdd490e38846
📝 ryanofsky opened a pull request: "Update libmultiprocess library"
(https://github.com/bitcoin/bitcoin/pull/31105)
Add recent changes and fixes for shutdown bugs.
https://github.com/chaincodelabs/libmultiprocess/pull/111: doc: Add internal design section
https://github.com/chaincodelabs/libmultiprocess/pull/113: Add missing include to util.h
https://github.com/chaincodelabs/libmultiprocess/pull/116: shutdown bugfix: destroy RPC system before running cleanup callbacks
https://github.com/chaincodelabs/libmultiprocess/pull/118: shutdown bugfix: Prevent segfault in server if connection is broken during lon
...
(https://github.com/bitcoin/bitcoin/pull/31105)
Add recent changes and fixes for shutdown bugs.
https://github.com/chaincodelabs/libmultiprocess/pull/111: doc: Add internal design section
https://github.com/chaincodelabs/libmultiprocess/pull/113: Add missing include to util.h
https://github.com/chaincodelabs/libmultiprocess/pull/116: shutdown bugfix: destroy RPC system before running cleanup callbacks
https://github.com/chaincodelabs/libmultiprocess/pull/118: shutdown bugfix: Prevent segfault in server if connection is broken during lon
...
👍 BrandonOdiwuor approved a pull request: "doc: remove dependency install instructions from win docs"
(https://github.com/bitcoin/bitcoin/pull/31100#pullrequestreview-2373066201)
ACK 184f12c1542f6c53eb2bd9dfb08dfdd490e38846
(https://github.com/bitcoin/bitcoin/pull/31100#pullrequestreview-2373066201)
ACK 184f12c1542f6c53eb2bd9dfb08dfdd490e38846
🤔 l0rinc reviewed a pull request: "Don't wipe coins cache when full and instead evict LRU clean entries"
(https://github.com/bitcoin/bitcoin/pull/31102#pullrequestreview-2373049420)
Since this depends on other PRs, would it make sense to draft it until those are merged and CI fixed?
(https://github.com/bitcoin/bitcoin/pull/31102#pullrequestreview-2373049420)
Since this depends on other PRs, would it make sense to draft it until those are merged and CI fixed?
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803458000)
it seemed to me that often we're only only checking the memory after we're finished with certain batches.
E.g starting `bitcoind` with `-dbcache=4` was still chugging along at 200k with `cache=128.7MiB`.
Would it be safer to lower this to 95%?
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803458000)
it seemed to me that often we're only only checking the memory after we're finished with certain batches.
E.g starting `bitcoind` with `-dbcache=4` was still chugging along at 200k with `cache=128.7MiB`.
Would it be safer to lower this to 95%?
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803459850)
formatting is really confusing here
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803459850)
formatting is really confusing here
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803460218)
this is very cheap, can we make it an assert instead?
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803460218)
this is very cheap, can we make it an assert instead?
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803462933)
same, formatting is inconsistent with `AddFlags` (where AddToLinkedList after the `if` is inside the condition, but here it's outside)
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803462933)
same, formatting is inconsistent with `AddFlags` (where AddToLinkedList after the `if` is inside the condition, but here it's outside)
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803449057)
nit: can we unify the formatting here?
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803449057)
nit: can we unify the formatting here?
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803460880)
`linked list` is an implementation detail, can we rather say the purpose of inclusion instead/
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803460880)
`linked list` is an implementation detail, can we rather say the purpose of inclusion instead/
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803466111)
could you please fix this in the commit that introduced the error instead?
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803466111)
could you please fix this in the commit that introduced the error instead?
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803468957)
the doc states this tries to evict entries - would it make sense to mention that instead of the memory cleaning effect of it?
(https://github.com/bitcoin/bitcoin/pull/31102#discussion_r1803468957)
the doc states this tries to evict entries - would it make sense to mention that instead of the memory cleaning effect of it?
💬 TheBlueMatt commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2417419737)
> The mining interface in interfaces/mining.h, node/interfaces.cpp and mining.capnp is orders of magnitude less code than what's currently in https://github.com/Sjors/bitcoin/pull/68. That would be still be true after a refactor of Sv2Connman.
I'm curious where the bulk of that is. It should be basically identical in terms of total volume (framing and such all has to happen either way, even if its auto-generated code when using capnproto), the protocol would be ~identical in function, and IPC
...
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2417419737)
> The mining interface in interfaces/mining.h, node/interfaces.cpp and mining.capnp is orders of magnitude less code than what's currently in https://github.com/Sjors/bitcoin/pull/68. That would be still be true after a refactor of Sv2Connman.
I'm curious where the bulk of that is. It should be basically identical in terms of total volume (framing and such all has to happen either way, even if its auto-generated code when using capnproto), the protocol would be ~identical in function, and IPC
...
💬 TheBlueMatt commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2417423369)
Its probably worth pointing out that with new MEV-extraction things coming out these days Bitcoin Core will not be the only server of work, which means we also need to define new mining work-providing protocols in terms of the server-side behavior as well (not just "whatever Bitcoin Core does", and not just whatever goes in its docs).
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2417423369)
Its probably worth pointing out that with new MEV-extraction things coming out these days Bitcoin Core will not be the only server of work, which means we also need to define new mining work-providing protocols in terms of the server-side behavior as well (not just "whatever Bitcoin Core does", and not just whatever goes in its docs).