Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 kevkevinpal commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1817630156)
I removed them in [1fab1d6](https://github.com/bitcoin/bitcoin/pull/31139/commits/1fab1d601e40edc7296cf3d60a7e917b0e6493b3)
💬 kevkevinpal commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1817631565)
ok I added an assert to check for `txid` in `getrawmempool` in [1fab1d6](https://github.com/bitcoin/bitcoin/pull/31139/commits/1fab1d601e40edc7296cf3d60a7e917b0e6493b3)

let me know if that looks good to you!
👍 hodlinator approved a pull request: "tinyformat: enforce compile-time checks for format string literals"
(https://github.com/bitcoin/bitcoin/pull/31149#pullrequestreview-2397082398)
re-ACK 23560b468c474bbc6a3abd6c0778da62766ac8f7

Changes since [previous ACK](https://github.com/bitcoin/bitcoin/pull/31149#pullrequestreview-2393618400):
- Added file-local `fuzz_fmt` function to reduce churn.
- Rebased and applied conversions to new code from #29936.
👍 rkrux approved a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2397147428)
ACK 1fab1d601e40edc7296cf3d60a7e917b0e6493b3

Thanks for incorporating the changes.
🤔 sdaftuar reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2395962778)
Looks pretty good, I think most of my comments are nitty. Along those lines, you may want to retouch the commit message of fcbfed4630bdee6c52644c89e972ba7c130f47d5, as I think there's a missing paren.
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817094524)
Is this logging correct? The `parent_txid` here is really the txid of the child that didn't spend all of some parent's dust. I don't think we know which actual parent of that transaction had the unspent dust, unless we add more information to the return value of `CheckEphemeralSpends`.
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817374367)
Just want to make sure I understand what you mean by "sibling" here. Is the idea that if we have a topology like this:

```mermaid
flowchart TD
TxA --> TxC
TxB --> TxC
```

Where Tx A has ephemeral dust outputs and is in the mempool, and Tx C spends them and is in the mempool, than you want to return an outpoint of Tx B, whether or not Tx B itself is in the mempool, and whether or not Tx B itself has ephemeral dust outputs?
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817315017)
I think all the parents in-mempool at this point?
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817772690)
Comment needs to be updated I think?
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817223902)
Is it also worth testing that this fails if submitted via `sendrawtransaction` as well, so we're not just testing the `AcceptPackage()` code path?
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817771844)
If it's not too hard, I was thinking it would be nice to have this fuzz test cover the package RBF case as well.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1817839632)
Fixed, thanks.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1817839644)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2439552457)
Rebased.
👍 TheCharlatan approved a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042#pullrequestreview-2397236697)
ACK 36a3de0af9a9120aa98a07ca4abbb77ee6e58ef9

The last two commits could be squashed together though? I don't think that would complicate the scripted diff.
💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2439562411)
> The last two commits could be squashed together though? I don't think that would complicate the scripted diff.

Thanks! Squashed.
👍 TheCharlatan approved a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042#pullrequestreview-2397244727)
Re-ACK 2076a0863ce6e7fcd5a1f2c29c38c1305816e57d
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r1817847707)
This would make sense if we wanted to reuse this for CCheckQueue. Then, we could just loop and `ProcessTask` on the main thread once when we call `Wait`.
🤔 glozow reviewed a pull request: "functional test: Additional package evaluation coverage"
(https://github.com/bitcoin/bitcoin/pull/31152#pullrequestreview-2397255278)
reACK f32c34d0c3d4041a301822b27e88d6db4cbf631e