Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 mzumsande reviewed a pull request: "net_processing: make any misbehavior trigger immediate discouragement"
(https://github.com/bitcoin/bitcoin/pull/29575#pullrequestreview-2123526970)
Code Review ACK 6eecba475efd025eb011400af58621ad5823994e
💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2174002062)
Force-push: rebased to master for qt version bump. Trivial changes to depends, didn't need to change qt patch.
💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1643213495)
@S3RK , @furszy you're right: `get_if` works here, as well. For some reason I was thinking `std::get_if` doesn't check if the type exists in the `std::variant` at compile time, hence using a visitor was better. But `std::get_if` does check that the type is included in the variant; its just that the visitor can actually do compile time checks to ensure all the cases are covered. But in the silent payments example, we don't need exhaustive coverage so the visitor is likely overkill. I've updated t
...
💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1643215137)
IIRC, I tried to move `CRecipient` out of `wallet.h` awhile back and it ended up being more complicated than I expected? But in general I agree.
💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1643215401)
Fixed.
👍 rkrux approved a pull request: "test: expand LimitOrphan and EraseForPeer coverage"
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2123601945)
reACK [172c1ad](https://github.com/bitcoin/bitcoin/pull/30082/commits/172c1ad026cc38c6f52679e74c14579ecc77c48e)

Thanks for very quickly addressing my comments @instagibbs, appreciate it!
💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#issuecomment-2174120310)
Updated to remove the visitor per @S3RK and @furszy 's feedback
💬 kevkevinpal commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#issuecomment-2174125844)
tACK [ad06e68](https://github.com/bitcoin/bitcoin/pull/30291/commits/ad06e68399da71c615db0dbf5304d0cd46bc1f40)

Looks good to me!
📝 instagibbs opened a pull request: "NOMERGE #28984 package rbf followups"
(https://github.com/bitcoin/bitcoin/pull/30295)
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643277261)
done in follow-up
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643277352)
done in follow-up
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643277672)
done in follow-up, I had removed V3 in test, not the actual doc
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643277948)
done in follow-up
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643278127)
wrote a better boundary test in this vein in follow-up, thanks. You actually need incremental added, not 1 sat.
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643278301)
took as inspiration to make an actual boundary-condition test in follow-up, thanks
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2174159709)
opened followup at https://github.com/bitcoin/bitcoin/pull/30295
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643279044)
done in follow-up
💬 achow101 commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#issuecomment-2174186974)
ACK ad06e68399da71c615db0dbf5304d0cd46bc1f40
💬 achow101 commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643298389)
In 0aa7db42564408edb41b0d42103d39ba4c2787dc "Validate oversized transaction"

I think it would be more interesting for this test to be on the threshold exactly. The rest of the tx is 70 bytes, so `MAX_BLOCK_WEIGHT / WITNESS_SCALE_FACTOR - 70` should pass `CheckTransaction`, while `- 69` does not.
🚀 achow101 merged a pull request: "test: write functional test results to csv"
(https://github.com/bitcoin/bitcoin/pull/30291)