💬 levantah commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2066470976)
> Possibly the best estimate we can get for that is to look at full-RBF replacements that do not get mined. It's not too uncommon to see, for example, OpenTimestamp's full-RBF replacements mysteriously get mined at the second or third highest fee-rate they were propagated at even though multiple minutes have passed since the higher fee-rate txs were broadcast. I believe that is evidence of poor propagation to miners on occasion.

Just a reminder that there is still this "too many replacements"
...

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2066473350)
Ok ready for review again.

💬 maflcko commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2066477567)
> Don't we have plethora of well documented guides and specifications on the JSON-RPC api ?

No. I'd highly doubt that there is a better source of the documentation and specification than Bitcoin Core itself. (If there is, then that is a bug in Bitcoin Core, which should be fixed)

> https://developer.bitcoin.org/reference/rpc/

This example has many issues:

* It is not machine readable
* It does not indicate the version of Bitcoin Core that is documented
* It is wrong and outdated. F
...

💬 maflcko commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#discussion_r1572306247)
Conceptually, this seems the wrong place for the fix. It would be nice if this was handled internally in chainman correctly.

Also, given that cs_main is held only for this line, a race exits, where the `getchainstates` RPC is called before this line, and still returns the wrong result.

💬 furszy commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#discussion_r1572336276)
> Conceptually, this seems the wrong place for the fix. It would be nice if this was handled internally in chainman correctly.

I did that initially (see #26260 - https://github.com/bitcoin/bitcoin/pull/26260#issuecomment-1270121139) but.. conceptually, all what is inside `reconsiderblock` seems to be at the wrong place for me too. We are resetting a block index flag at the RPC level. And, ideally, we should encapsulate all this code within chainman too.

Also, see https://github.com/bitcoin
...

💬 hebasto commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2066565398)
> We can cherry-pick one commit from upstream leveldb, make the same change in crc32c, and then ultimately drop our build infra for testing endianness.

The same goal, which is dropping "build infra for testing endianness", might be achieved with an [alternative](https://github.com/hebasto/bitcoin/commit/ee5096ac4e638f088c78850a5d2e401fbe88537f) approach, which essentially boils down to:
```diff
--- a/src/leveldb/util/coding.h
+++ b/src/leveldb/util/coding.h
@@ -62,7 +62,7 @@ char* EncodeV
...

👍 vasild approved a pull request: "net: attempts to connect to all resolved addresses when connecting to a node"
(https://github.com/bitcoin/bitcoin/pull/28834#pullrequestreview-2011406209)
ACK fd81a37239541d0d508402cd4eeb28f38128c1f2

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1572363409)
pushed updated text

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1572364608)
added a small check here using `GetEntry` that takes `Wtxid`s.

💬 fanquake commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2066586806)
I don't think that's better. We have to keep all the redundant code, and now we are diveraging from upstream for no reason.

💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-2066602344)
@maflcko @hernanmarino sorry I misunderstood https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1956896591 earlier

When the wallet is disabled in the configure, the wallet RPCs are not included in the `rpc_interface.txt` list, hence not part of the coverage check


(example below from my test)
[rpc_interface.txt](https://github.com/bitcoin/bitcoin/files/15041102/rpc_interface.txt)

💬 fanquake commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2066606404)
Also, you're just moving the endianess testing into the code. The point is to drop all of this, and use generic code that doesn't require any tests at all. The fact that MSVC fails to perform basic optimisations is annoying, but I don't really see why it's a blocker here. If we'd just pulled the subtree, and this change came in as part of that, I doubt anyone would have even noticed anything MSVC related (still haven't seen any benchmarks showing any meaningful difference for this change as-is)?
...

💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2066644693)
Updates:
- I've been working on @jonatack's latest feedback, I'm performing some tests at the moment and will update the code soon.

👍 instagibbs approved a pull request: "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)"
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-2011520053)
ACK 60ca5d55081275a011ccfc9546e0c4a8c4030493

please don't feel obligated to touch due to nit

💬 instagibbs commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1572424009)
micro-nit: this line can be removed safely now :partying_face:

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1572436712)
duplicated `cleanup`, or something?

💬 achow101 commented on issue "DNS seed "seed.bitcoinstats.com" doesn't support filtering while the comments says it does":
(https://github.com/bitcoin/bitcoin/issues/29911#issuecomment-2066674660)
cc @cdecker

💬 achow101 commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2066677901)
Note that automatically generated RPC docs are also available for each major version at https://bitcoincore.org/en/doc/

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1572451256)
Ah forgot to delete, yes

💬 furszy commented on issue "importdescriptors hanging on importing/updating descriptor with large range":
(https://github.com/bitcoin/bitcoin/issues/25895#issuecomment-2066698109)
> @furszy is this fixed in master?

Locally, the test doesn't take much. Around 100 secs. But I can see how it could take a while on a spinning disk. #28894 should have improved the import situation but we are still far from getting #25297.
Once #28574 gets merged, will continue decoupling #25297 in smaller PRs to improve this scenario.