💬 paplorinc commented on pull request "fuzz: Speed up PickValue in txorphan":
(https://github.com/bitcoin/bitcoin/pull/30474#discussion_r1682881489)
I means simply:
```patch
===================================================================
diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
--- a/src/test/fuzz/txorphan.cpp (revision 37992244e636e52e0c2baff1bc5f36e60d9c956f)
+++ b/src/test/fuzz/txorphan.cpp (date 1721306232235)
@@ -37,11 +37,11 @@
SetMockTime(ConsumeTime(fuzzed_data_provider));

TxOrphanage orphanage;
- std::set<COutPoint> outpoints;
+ std::vector<COutPoint> outpoints;

/
...

💬 maflcko commented on pull request "refactor: Add consteval uint256("str") and ParseHex("str")":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1682882026)
Also, I wonder if the three duplicate redirects can be replaced by a single `uinsg base_blob::base_blob;` (or similar) to import all constructors.

💬 maflcko commented on pull request "fuzz: Speed up PickValue in txorphan":
(https://github.com/bitcoin/bitcoin/pull/30474#discussion_r1682889408)
Yeah, I guess, given that duplicates are tolerated either way

👍 theStack approved a pull request: "test: Non-Shy version sender"
(https://github.com/bitcoin/bitcoin/pull/30453#pullrequestreview-2185928833)
tACK faed5d3870fb32fb5278961ee23e38fd9ea6ca15

Thanks for following up! Tested this with the following patch, where the introduced debug messages would pop up (with received message type "version" as expected) in the log every few runs:

<details>
<summary>Patch</summary>

```diff
diff --git a/src/net.h b/src/net.h
index a4e4b50360..8b9b0003b8 100644
--- a/src/net.h
+++ b/src/net.h
@@ -747,6 +747,13 @@ public:
std::optional<std::pair<CNetMessage, bool>> PollMessage()

...

💬 maflcko commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2236633461)
> Electrum have adopted a practical approach, but it's not entirely correct. They consider all conflicting transactions, and then discard according to the hierarchy confirmed > mempool > local ([reference](https://github.com/spesmilo/electrum/blob/76f5d6ceb07b49e403f63488832fd6cb397ee91b/electrum/address_synchronizer.py#L295-L298)). This does not consider policy rules, and may result in discarding a transaction that is later mined.

Yes, but that should be harmless, as a confirmed transaction
...

💬 glozow commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2236638869)
concept ACK

💬 maflcko commented on pull request "test: Non-Shy version sender":
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2236639377)
> I think thematically the test would also fit very well into `p2p_handshake.py`, but no strong feelings.

Sure, I may move-only it, if I have to re-touch for other reasons.

⚠️ luisschwab opened an issue: "`scantxoutset` should output the block hash instead of block height"
(https://github.com/bitcoin/bitcoin/issues/30478)
### Please describe the feature you'd like to see added.

When using the `scantxoutset` RPC, the current behaviour is to show the block **height** of the UTXO. This is not optimal, as block height is ambiguous, especially in the case of a block reorganization happening at the same instant of the query. In this case, an UTXO that does not exist would be assumed to exist, unless the chain's tip hash is recorded before the scan, and make sure it still exists after, as per [this](https://github.com/
...

💬 fanquake commented on pull request "depends: build FreeType with CMake":
(https://github.com/bitcoin/bitcoin/pull/29880#issuecomment-2236654832)
No-longer a version bump. Just a switch from Autotools -> CMake.

👋 fanquake's pull request is ready for review: "depends: build FreeType with CMake"
(https://github.com/bitcoin/bitcoin/pull/29880)

💬 maflcko commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236660706)
> This is not optimal, as block height is ambiguous, especially in the case of a block reorganization happening at the same instant of the query.

Sounds like a bug. The code:


```cpp
{
ChainstateManager& chainman = EnsureChainman(node);
LOCK(cs_main);
Chainstate& active_chainstate = chainman.ActiveChainstate();
active_chainstate.ForceFlushStateToDisk();
pcursor = CHECK_NONFATAL(active_chainstate.CoinsDB().Cursor());
...

🤔 danielabrozzoni reviewed a pull request: "rest: Reject negative outpoint index early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30444#pullrequestreview-2185988177)
ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1

I checked manually that `+` and `-` in `getutxos` calls were once accepted and are now rejected.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#issuecomment-2236683680)
Yes, makes sense.

💬 maflcko commented on pull request "refactor: Add consteval uint256("str") and ParseHex("str")":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1682962565)
Not sure about these. It should fail compilation instead if non-hex is detected.

💬 petertodd commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2236753415)
Concept NACK

There's no reason to limit this behavior to TRUC/V3 transactions. The implementation should not ship with this restriction.

💬 maflcko commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236758192)
Sorry, no. The code is fine, because the cursor and the tip are taken in the same lock.

And the height and hash are already returned. Not sure what is left to be done here.

I don't see the value of removing an RPC field and breaking clients for no clear reason.

I understand that the current docs for the fields may not be ideal. So pull requests with improvements are welcome :)

💬 sipa commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236762902)
@luisschwab Just to be clear, I believe that the block hash you're asking for is already returned, under the name `"bestblock"`. I don't think we should be removing the height, but you're of course free to ignore it.

🚀 ryanofsky merged a pull request: "refactor: add coinbase constraints to BlockAssembler::Options"
(https://github.com/bitcoin/bitcoin/pull/30356)

💬 sipa commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236771031)
Actually, I think there is some confusion here.

@luisschwab is talking about the height/blockhash of the **UTXO** found, but the suggested diff in RPC output is about the height/blockhash of the **tip** up to where the scan occurred.

I don't think it would be hard to add a block hash to the individual UTXO records returned by `scantxoutset` (for the block they were created in), in addition to the `"height"` field returned for them.

💬 craigraw commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2236775996)
> Yes, but that should be harmless, as a confirmed transaction in the chain can be retrieved from the chain. The same problem would exist in a broadcast pool, because any transaction in it is not guaranteed to be confirmed, and an earlier (or later) replacement could be confirmed, or none at all.

I don't agree - it could lead to a situation where a user is fee bumping a transaction unnecessarily. Certainly there are no guarantees on confirmation, but where a transaction is discarded locally i
...