💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1580798337)
You moved `fill_mempool` to util in #29735

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1579508562)
Question: here you mean the package RBF rules in OP not BIP125 right?

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1580946063)
nit:
```suggestion
def test_package_rbf_with_numerous_ancestors(self):
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1580918022)
The parent transaction will be detected as `inmempool` as such the child just get added to the mempool if it passes normal transaction consensus and policy rules, the package does not have any conflict and hence package RBF rules are not checked.

This test also passes when I removed package RBF commits
```suggestion
def test_submitpackage_with_a_mempool_parent(self):
self.log.info("Test that submitpackage works when parent transactions was already submitted")
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1580973594)
```suggestion
# Package 2 feerate is below the feerate of directly conflicted parent, even though
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1579749486)
`package1` and `package3` has the same parent

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1580970962)
nit:
```suggestion
def test_package_rbf_with_wrong_pkg_size(self):
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1581088045)
nice this will be helpful for collecting data, previously we only know that a transaction is replaced now it indicates which tx replaced it!

E.g a transaction evicting two transaction package evicting another two
```terminal
2024-04-26T14:36:53.889903Z [httpworker.3] [validation.cpp:1282] [Finalize] [mempool] replacing mempool tx 1ae8118ceb71249b2af793f671a1293a4c999e89f5091f52c42a3cf1e08c6840 (wtxid=97fc8654dcbd025b4165c56179d124ee4adf2d65acf8a66f8f5464dab7d07089, fees=10000, vsize=104). N
...

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1581185454)
Ok, agreed

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2079604399)
> ACK [be2c551](https://github.com/bitcoin/bitcoin/commit/be2c5510521081119f62ef3b29f76bb9097d0f34) Re-reviewed code, and attempted to break tests by making `ConsiderEviction` misbehave.
>
> It is worth noting that these tests will not catch if outbound eviction logic gets triggered too early. e.g.:
>
> ```c++
> /** How long to wait for a peer to respond to a getheaders request */
> static constexpr auto HEADERS_RESPONSE_TIME{0s};
> // [...]
> /** Timeout for (unprotected) outbound pee
...

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2079606752)
Updated to address https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1580221364

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-2079616145)
re: https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-2079388456

> > or the [CompleteChainstateInitialization](https://github.com/ryanofsky/bitcoin/blob/55d7de92bbbe035c1833c89f885af14e5b243932/src/node/chainstate.cpp#L38-L179) function from #25665
>
> I don't think this is good usage of `Update()`, since there is no chaining happening here. I would find list initialization more readable and less error-prone:
>
> git diff on 1376583

The diff you suggested is actually the way
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2079619223)
I'm not a Rust guru, but if someone can write a quick script to make `rust-silentpayments` spit out the tweaks per block, I can compare it with our index.

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#discussion_r1581204910)
Ok, I've pushed a commit. Let me know what you think.

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#discussion_r1581205666)
Not sure what to do here. I'll resolve this conversation for now.

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#discussion_r1581206073)
Fixed, so resolving this for now.

📝 achow101 locked a pull request: "fix block subsidy at 3.25 BTC"
(https://github.com/bitcoin/bitcoin/pull/29778)
This fixes block subsidy at 3.25 BTC.
- ensures ongoing profitability of mining operations
- increasing money supply for growing Bitcoin economy
- to be rolled out after April 2024 halving

💬 achow101 commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2079680643)
Yes, this happens if you try to run any of the CI tasks locally in a worktree.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1581275198)
done

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1581275245)
taken