💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2239730901)
Ok, the rework ended up being quite easy.
Pushed an update that generalizes to non-TRUC transactions, but as noted earlier, individual transactions have to meet minrelay for now unless they are TRUC, see/divert discussion here for that specific point: https://github.com/bitcoin/bitcoin/pull/26933 . If this restriction goes away, it will be usable(see tests I added with v2 transactions at the bottom where I checked it). Note that we are still constrained to 1P1C relay until we get general pack
...
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2239730901)
Ok, the rework ended up being quite easy.
Pushed an update that generalizes to non-TRUC transactions, but as noted earlier, individual transactions have to meet minrelay for now unless they are TRUC, see/divert discussion here for that specific point: https://github.com/bitcoin/bitcoin/pull/26933 . If this restriction goes away, it will be usable(see tests I added with v2 transactions at the bottom where I checked it). Note that we are still constrained to 1P1C relay until we get general pack
...
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698131)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698131)
Done.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698616)
We don't invoke ExhaustiveCandidateFinder for larger clusters because it just gets too computationally expensive to do so. I've added a comment to clarify this.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698616)
We don't invoke ExhaustiveCandidateFinder for larger clusters because it just gets too computationally expensive to do so. I've added a comment to clarify this.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698702)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698702)
Done.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698780)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698780)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698872)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698872)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698968)
Did something like that.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698968)
Did something like that.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684699167)
I've added a comment to clarify.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684699167)
I've added a comment to clarify.
💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1684699856)
> Note: This line isn't directly equivalent to previous code because it trims whitespace from the end of the string, not just the beginning, but the net effect is the same because any whitespace at the end of the string would be ignored anyway.
@ryanofsky yes, that's why my first pushed implementation added `LeftTrimStringView` in 8f4293d892358ce145eb9a099f2e5f256c4d9c73. But got pushback in https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675658574 and trimming the end of the strin
...
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1684699856)
> Note: This line isn't directly equivalent to previous code because it trims whitespace from the end of the string, not just the beginning, but the net effect is the same because any whitespace at the end of the string would be ignored anyway.
@ryanofsky yes, that's why my first pushed implementation added `LeftTrimStringView` in 8f4293d892358ce145eb9a099f2e5f256c4d9c73. But got pushback in https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675658574 and trimming the end of the strin
...
📝 theuni opened a pull request: "depends: Fix CMake-generated `libevent*.pc` files"
(https://github.com/bitcoin/bitcoin/pull/30488)
Broken out of #30454. This is a backport of the merged upstream PR: https://github.com/libevent/libevent/pull/1622.
Note that after #29835 we might end up dropping pkg-config and using the installed CMake files directly, but that depends on whether or not enough distros actually ship those files.
Either way, having fixed up .pc files won't hurt.
(https://github.com/bitcoin/bitcoin/pull/30488)
Broken out of #30454. This is a backport of the merged upstream PR: https://github.com/libevent/libevent/pull/1622.
Note that after #29835 we might end up dropping pkg-config and using the installed CMake files directly, but that depends on whether or not enough distros actually ship those files.
Either way, having fixed up .pc files won't hurt.
👍 tdb3 approved a pull request: "test: add creating/spending validity checks for rare output scripts"
(https://github.com/bitcoin/bitcoin/pull/30481#pullrequestreview-2188793231)
ACK 006d835acf98ebceaaaead7aea41f512ed5023ad
Seems like a good addition. Observed a runtime of around 1s, so minimal.
Left some thoughts. Will also think about other potential candidate rare output scripts and comment if any come to mind.
(https://github.com/bitcoin/bitcoin/pull/30481#pullrequestreview-2188793231)
ACK 006d835acf98ebceaaaead7aea41f512ed5023ad
Seems like a good addition. Observed a runtime of around 1s, so minimal.
Left some thoughts. Will also think about other potential candidate rare output scripts and comment if any come to mind.
💬 tdb3 commented on pull request "test: add creating/spending validity checks for rare output scripts":
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1684707058)
Thinking out loud:
Initially thought of ensuring the edge cases (2 and 40) are explicitly added in addition to random program size selection, but this would probably be overkill.
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1684707058)
Thinking out loud:
Initially thought of ensuring the edge cases (2 and 40) are explicitly added in addition to random program size selection, but this would probably be overkill.
💬 tdb3 commented on pull request "test: add creating/spending validity checks for rare output scripts":
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1684710348)
Thinking out loud:
Might be nice to enhance ECPubKey to handle hybrid keys (prevents having to do byte manipulation here). Seems outside the scope of this PR though.
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1684710348)
Thinking out loud:
Might be nice to enhance ECPubKey to handle hybrid keys (prevents having to do byte manipulation here). Seems outside the scope of this PR though.
📝 theuni opened a pull request: "depends: zmq: Fix Autotools-generated `libzmq.pc` file"
(https://github.com/bitcoin/bitcoin/pull/30489)
Similar to #30488. Broken out of #30454. This is just taking the (merged) upstreamed PR: https://github.com/zeromq/libzmq/pull/4667
On top of #29723 as that would otherwise conflict and is likely to be merged soon. I'll rebase here and undraft after that's merged.
Like #30488, this may be obsoleted soon after switching to building with CMake because we'll be able to use those files rather than pkg-config, but again it doesn't hurt to have the fixed files.
(https://github.com/bitcoin/bitcoin/pull/30489)
Similar to #30488. Broken out of #30454. This is just taking the (merged) upstreamed PR: https://github.com/zeromq/libzmq/pull/4667
On top of #29723 as that would otherwise conflict and is likely to be merged soon. I'll rebase here and undraft after that's merged.
Like #30488, this may be obsoleted soon after switching to building with CMake because we'll be able to use those files rather than pkg-config, but again it doesn't hurt to have the fixed files.
✅ theuni closed a pull request: "depends: zmq: Fix Autotools-generated `libzmq.pc` file"
(https://github.com/bitcoin/bitcoin/pull/30489)
(https://github.com/bitcoin/bitcoin/pull/30489)
💬 theuni commented on pull request "depends: zmq: Fix Autotools-generated `libzmq.pc` file":
(https://github.com/bitcoin/bitcoin/pull/30489#issuecomment-2239844520)
Hah, I realized as soon as I hit the button that #29723 obsoletes this. Either the .pc is fine there, or it needs to be patched in a different place. Closing.
(https://github.com/bitcoin/bitcoin/pull/30489#issuecomment-2239844520)
Hah, I realized as soon as I hit the button that #29723 obsoletes this. Either the .pc is fine there, or it needs to be patched in a different place. Closing.
💬 theuni commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2239864170)
Looking at the mingw .pc generated by this PR:
```
Libs: -L${libdir} -lzmq
Libs.private:
Requires.private:
```
It looks like we'll need to take https://github.com/zeromq/libzmq/pull/4706 as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2239864170)
Looking at the mingw .pc generated by this PR:
```
Libs: -L${libdir} -lzmq
Libs.private:
Requires.private:
```
It looks like we'll need to take https://github.com/zeromq/libzmq/pull/4706 as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.
💬 theuni commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1684802528)
Looks like this can be bumped further now that https://github.com/chaincodelabs/libmultiprocess/pull/98 has been merged?
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1684802528)
Looks like this can be bumped further now that https://github.com/chaincodelabs/libmultiprocess/pull/98 has been merged?
📝 theuni opened a pull request: "depends: bump libmultiprocess for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30490)
Broken out of #30454 . Bumped [even further](https://github.com/bitcoin/bitcoin/pull/30454/commits/4883197abc63aedbc395f37f6d2aded5db5270aa#r1684802528) after https://github.com/chaincodelabs/libmultiprocess/pull/98 was merged upstream.
@hebasto Presumably this approach works now with the CMake branch?
(https://github.com/bitcoin/bitcoin/pull/30490)
Broken out of #30454 . Bumped [even further](https://github.com/bitcoin/bitcoin/pull/30454/commits/4883197abc63aedbc395f37f6d2aded5db5270aa#r1684802528) after https://github.com/chaincodelabs/libmultiprocess/pull/98 was merged upstream.
@hebasto Presumably this approach works now with the CMake branch?
💬 theuni commented on pull request "depends: bump libmultiprocess for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30490#issuecomment-2239894232)
Ping @ryanofsky for a quick concept ACK for bumping to this particular commit.
(https://github.com/bitcoin/bitcoin/pull/30490#issuecomment-2239894232)
Ping @ryanofsky for a quick concept ACK for bumping to this particular commit.