💬 petertodd commented on pull request "[WIP] BIP300 (Drivechains) consensus-level logic":
(https://github.com/bitcoin/bitcoin/pull/28311#issuecomment-1824382711)
> @petertodd
>
> > Note that large coinbase transactions have caused a number of problems before,
> > including issues with ASIC compatibility and p2pool. We should not inflate
> > coinbase transaction size needlessly.
>
> What responsibilities going forward do devs have with asic compatibility? Seems like a potential conflict of interest in some situations.

Obviously we should not do things that are incompatible with existing ASICs without a *very* good reason. A feature that doesn't
...

💬 furszy commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1403356154)
Sure. Done as suggested.

💬 furszy commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1403366367)
Yeah, I was thinking about https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1401323753 and other features I have in mind. But, more generally, I believe the wallet should be aware of the source of the event. However, upon further thought, we could achieve this by creating multiple handlers on the wallet side instead of passing the origin pointer to the signal itself.

💬 furszy commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#issuecomment-1824411647)
Updated per feedback. Tiny [diff](https://github.com/bitcoin/bitcoin/compare/44e2c744b8bae075da69d8e8dd392296899a68a5..25de6ff824bffd8617a6baca19bbe57d6764a9d2).
Only added a small explanatory comment.

💬 Sjors commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1824503673)
Just ran into this today while testing #27463.

To make testing of package submission easier, should we have an option to allow this on test networks? Or perhaps a way to set a minimum for `GetMinFee()`?

@sdaftuar does mempool clustering fix this? E.g. is it just a matter of (occasionally) evicting chunks below `-minrelaytxfee`? From the above discussion I get the impression that the answer is yes, but most of this was discussed before that proposal came out.

🤔 furszy reviewed a pull request: "test: Add gettransaction test for "coin-join" tx"
(https://github.com/bitcoin/bitcoin/pull/18919#pullrequestreview-1746552710)
utACK fa20f8919c4f

💬 furszy commented on pull request "test: Add gettransaction test for "coin-join" tx":
(https://github.com/bitcoin/bitcoin/pull/18919#discussion_r1403442586)
Could we add a "todo:" at the beginning?
Just to make easier to find broken things in the future.

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1824513479)
@Sjors testing it includes making sure nothing below minrelay can enter the mempool, unfortunately until v3(where 0-fee will be allowed) or cluster mempool is deployed, which makes trimming very fast

💬 instagibbs commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#issuecomment-1824515731)
@Sjors I think you can set `-minrelaytxfee=0` to work around it?

💬 maflcko commented on pull request "test: Add gettransaction test for "coin-join" tx":
(https://github.com/bitcoin/bitcoin/pull/18919#discussion_r1403457573)
I am not a fan of TODO comments, because I think issues in the issue tracker are a better way to track outstanding feature requests and bugfixes. An issue allows anyone to leave new context, ask questions, or look up the full discussion history on the topic.

So going to leave as-is for now.

💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1403481849)
Do you remember why you added a `do { ... } while (0)` to the `TRACEPOINT` macro?

💬 Sjors commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#issuecomment-1824570747)
> @Sjors I think you can set `-minrelaytxfee=0` to work around it?

That doesn't simulate a full mempool though.

I wrote a commit that lets my example go through: https://github.com/Sjors/bitcoin/commit/03baacd7643daf3d1c8efbc1a723719d8cc72dce

However, not sure if it's worth the extra complexity.

🤔 Sjors reviewed a pull request: "bugfix, Change up submitpackage results to return results for all transactions"
(https://github.com/bitcoin/bitcoin/pull/28848#pullrequestreview-1746652527)
Anyway, my two main issues are about pre-existing behavior: `tx-results` was already a dictionary and `-minrelaytxfee` already behaved that way.

d72aab3dbd8847e851f8b724b195181e30e7bf02 looks good, but I'm (obviously) not familiar enough with mempool stuff to know for sure.

💬 Sjors commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1403506481)
Do you still want to check "mempool full" like below?

📝 BrandonOdiwuor opened a pull request: "wallet: Add checkbalance RPC"
(https://github.com/bitcoin/bitcoin/pull/28930)
Fixes https://github.com/bitcoin/bitcoin/issues/28898

💬 maflcko commented on pull request "wallet: Add checkbalance RPC":
(https://github.com/bitcoin/bitcoin/pull/28930#issuecomment-1824634741)
Could add tests for:
* Tx in mempool
* Watchonly wallet
* ...

?

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1824696971)
Another one:

[clusterfuzz-testcase-script_flags-4564162336129024.not.txt](https://github.com/bitcoin/bitcoin/files/13452132/clusterfuzz-testcase-script_flags-4564162336129024.not.txt)


💬 fjahr commented on pull request "contrib: add test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1824718897)
Concept ACK

I think this could use some documentation on top of the file on how the script is intended to be used, see the text in `test_utxo_snapshots.sh`.

📝 maflcko opened a pull request: "fuzz: Limit fuzz buffer size in script_flags target"
(https://github.com/bitcoin/bitcoin/pull/28931)
Most fuzz targets have an upper limit on the buffer size to avoid excessive runtime. Do the same for `script_flags` to avoid timeouts such as https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1824696971

Also, fix iwyu. Also, remove legacy `CDataStream`.

🚀 fanquake merged a pull request: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578)