📝 BrandonOdiwuor converted_to_draft a pull request: "Wallet listreceivedby fix"
(https://github.com/bitcoin/bitcoin/pull/30972)
Fixes https://github.com/bitcoin/bitcoin/issues/16159,
This PR builds on https://github.com/bitcoin/bitcoin/pull/25973, fixing `listreceivedby*` RPCs by filtering out `send` addresses using `IsMine` (see https://github.com/bitcoin/bitcoin/pull/25973#discussion_r1269477246). It also breaks down the `listreceivedby` tests into subtests and adds a test to verify 'listreceivedby*' does not return `send` addresses
(https://github.com/bitcoin/bitcoin/pull/30972)
Fixes https://github.com/bitcoin/bitcoin/issues/16159,
This PR builds on https://github.com/bitcoin/bitcoin/pull/25973, fixing `listreceivedby*` RPCs by filtering out `send` addresses using `IsMine` (see https://github.com/bitcoin/bitcoin/pull/25973#discussion_r1269477246). It also breaks down the `listreceivedby` tests into subtests and adds a test to verify 'listreceivedby*' does not return `send` addresses
💬 average-gary commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374720112)
Is there no way to query AS Maps directly from the BGP tables?
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374720112)
Is there no way to query AS Maps directly from the BGP tables?
💬 TheCharlatan commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2374760380)
Thank you for your suggestion @ryanofsky,
Updated 705105de4f84f2ce3bb1d754c88c32349e01bb3b -> e9d60af9889c12b4d427adefa53fd234e417f8f6 ([removeInitForLoop_0](https://github.com/TheCharlatan/bitcoin/tree/removeInitForLoop_0) -> [removeInitForLoop_1](https://github.com/TheCharlatan/bitcoin/tree/removeInitForLoop_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeInitForLoop_0..removeInitForLoop_1))
* Took @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/3096
...
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2374760380)
Thank you for your suggestion @ryanofsky,
Updated 705105de4f84f2ce3bb1d754c88c32349e01bb3b -> e9d60af9889c12b4d427adefa53fd234e417f8f6 ([removeInitForLoop_0](https://github.com/TheCharlatan/bitcoin/tree/removeInitForLoop_0) -> [removeInitForLoop_1](https://github.com/TheCharlatan/bitcoin/tree/removeInitForLoop_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeInitForLoop_0..removeInitForLoop_1))
* Took @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/3096
...
💬 sdaftuar commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2374765971)
You're right that rule 2 is broken. The issue is that our replacement rules are broken in many ways, and if we simply remove rule 2, the result would also be broken. The motivation for rule 2 was the idea that by introducing a new unconfirmed parent, the effective feerate at which the replacement transaction being validated would be mined might be much lower than the transactions it is evicting (say, if the new unconfirmed parent had a very low feerate). So by simply removing the rule, I thin
...
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2374765971)
You're right that rule 2 is broken. The issue is that our replacement rules are broken in many ways, and if we simply remove rule 2, the result would also be broken. The motivation for rule 2 was the idea that by introducing a new unconfirmed parent, the effective feerate at which the replacement transaction being validated would be mined might be much lower than the transactions it is evicting (say, if the new unconfirmed parent had a very low feerate). So by simply removing the rule, I thin
...
🤔 ismaelsadeeq reviewed a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2328851273)
Code review bf1b1b09ca4dd18b7848fb1807df8533cf930df7
This is a nice refactor, really improve the cluster linearize test a lot.
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2328851273)
Code review bf1b1b09ca4dd18b7848fb1807df8533cf930df7
This is a nice refactor, really improve the cluster linearize test a lot.
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775578705)
I believe this new behavior is correct, as it reduces `iteration_left` when work is being done. This aligns with the behavior in `SearchCandidateFinder`.
On a side note, I think "iterations" might be a misnomer here. A more accurate term could be "maximum optimization" or "optimizations left".
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775578705)
I believe this new behavior is correct, as it reduces `iteration_left` when work is being done. This aligns with the behavior in `SearchCandidateFinder`.
On a side note, I think "iterations" might be a misnomer here. A more accurate term could be "maximum optimization" or "optimizations left".
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775638585)
This can be in the same loop above?
Why not just compare with the result of ancestor finder?
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775638585)
This can be in the same loop above?
Why not just compare with the result of ancestor finder?
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775677456)
Should we also assert that the transactions in the set are the same?
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775677456)
Should we also assert that the transactions in the set are the same?
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775651129)
We could wrap this in a helper to be dry, as same code is used above.
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775651129)
We could wrap this in a helper to be dry, as same code is used above.
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775682420)
Maybe indicate that we only compare `SimpleCandidateFinder` with `ExhaustiveCandidateFinder` when we optimally found a subset and the cluster size is small.
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775682420)
Maybe indicate that we only compare `SimpleCandidateFinder` with `ExhaustiveCandidateFinder` when we optimally found a subset and the cluster size is small.
🤔 ismaelsadeeq reviewed a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2329121486)
Nice Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2329121486)
Nice Concept ACK
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775732905)
could be in it's own commit?
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775732905)
could be in it's own commit?
💬 achow101 commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374863380)
ACK c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374863380)
ACK c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee
✅ achow101 closed an issue: "`invalidateblock` during background validation"
(https://github.com/bitcoin/bitcoin/issues/30958)
(https://github.com/bitcoin/bitcoin/issues/30958)
🚀 achow101 merged a pull request: "validation: Disable CheckForkWarningConditions for background chainstate"
(https://github.com/bitcoin/bitcoin/pull/30962)
(https://github.com/bitcoin/bitcoin/pull/30962)
💬 Oly6 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/39219fe145e5e6e6f079b591e3f4b5fea8e71804#commitcomment-147202742)
good
(https://github.com/bitcoin/bitcoin/commit/39219fe145e5e6e6f079b591e3f4b5fea8e71804#commitcomment-147202742)
good
💬 achow101 commented on pull request "[28.x] backports and finalize (or rc3)":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2374879207)
Added #30962
I think it's trivial enough to not require rc3 as well.
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2374879207)
Added #30962
I think it's trivial enough to not require rc3 as well.
💬 achow101 commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374879640)
Backport in #30959
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374879640)
Backport in #30959
💬 fjahr commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374881703)
> Is there no way to query AS Maps directly from the BGP tables?
Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374881703)
> Is there no way to query AS Maps directly from the BGP tables?
Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?
🤔 ryanofsky reviewed a pull request: "refactor: Implement missing error checking for ArgsManager flags"
(https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2329185957)
Updated 87b6d4cb5a5c04f6c8542c633c3bfa5f76901d43 -> 5a945600451037693a032e6df94f99a666dd581f ([`pr/argcheck.41`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.41) -> [`pr/argcheck.42`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.42), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/argcheck.41..pr/argcheck.42)) with suggested formatting & grammar tweaks.
re: https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2325050760
Thanks for the review. To
...
(https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2329185957)
Updated 87b6d4cb5a5c04f6c8542c633c3bfa5f76901d43 -> 5a945600451037693a032e6df94f99a666dd581f ([`pr/argcheck.41`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.41) -> [`pr/argcheck.42`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.42), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/argcheck.41..pr/argcheck.42)) with suggested formatting & grammar tweaks.
re: https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2325050760
Thanks for the review. To
...