💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439769121)
The current approach addresses overflow concerns as per https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1411821098

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1874485977)
> This discussion is not about general Bitcoin protocol design. The question is if V3 transactions actually solves a real problem in a real protocol, thus making it a worthwhile addition to Bitcoin Core.

V3 is useful for any transactions where you don't want to be RBF-pinned for sending to arbitrary addresses. For example in splicing, rather than requiring each address being sent to including a `1 OP_CSV` clause over each spending condition, it can now be arbitrary scriptPubKeys. More general
...

💬 mzumsande commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439781598)
ok, I see. Still, having a function that returns 3 as the medium of {3,5} and 5 as the medium of {4,6} seems inconsistent, couldn't we explicitly check and handle overflows instead?

💬 darosior commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1874508653)
> You've misunderstood how Lightning works. While simplified explanations often talk about [...]

I see how a superficial understanding of Lightning and pinning issues could lead you to think that. However your hot fix to your proposal does not patch all its flaws.

> In the context of RBF using channels — while there may be special circumstances requiring a different allocation — the most obvious way to allocate fees is to take the fees from the balance on each respective side. So if Alice
...

💬 darosior commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1439796296)
I haven't. Can we use a ramdisk at oss-fuzz?

💬 hebasto commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1439881726)
> Looks like the macOS CI now fails for unrelated reasons. If someone created a separate pull request to bump xcode in the macOS CI, that'd be great.

The underlying issue is that Apple Clang 15 does not support `-Xclang -internal-isystem` anymore. Therefore, supporting for the `--enable-suppress-external-warnings` option on `x86_64` macOS becomes a non-trivial task.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1439887042)
Indeed, fixed.

💬 achow101 commented on pull request "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs":
(https://github.com/bitcoin/bitcoin/pull/28868#discussion_r1439890323)
Done as suggested

💬 achow101 commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-1874681373)
> Guessing that in the case of adding a new type of encrypted record in the future for private keys disabled wallets, we would be forced to add downgrade protection if we merge this PR?

Yes, but I think it's unlikely such usage of encryption keys would be added.

💬 BTCMcBoatface commented on pull request "Make provably unsignable standard P2PK and P2MS outpoints unspendable.":
(https://github.com/bitcoin/bitcoin/pull/28400#issuecomment-1874684848)
I have no business adding comments here but I may humbly add that, despite this only affecting 0.016% of the UTXOs, it is a smart preemptive defense against arbitrary data on chain using a method that is far more damaging than inscriptions.

💬 achow101 commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#issuecomment-1874716791)
> Also, while was working on this, discovered another deadlocking cause.. test [furszy@68e77b5](https://github.com/furszy/bitcoin-core/commit/68e77b58e2284a92daff49a324fd229497da37ae). As it is a bdb-only issue, we could address it in another PR. But, it will mean that [furszy@a80cc56](https://github.com/furszy/bitcoin-core/commit/a80cc56f0d1bdc82e9458f957db03c08620a6dd7) will need to be adapted to only run on sqlite (replace `TestDatabases()` for sqlite db creation call).

Hmm, seems like BDB
...

💬 achow101 commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1439975361)
Fixed

💬 petertodd commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1874723441)
> As noted in delving, pinning is not solved for anchor channels(even if we assume package relay/rbf is implemented): If an adversary splits the view of which commitment transaction is broadcasted, the remote copy can become the pin since the defender is unable to propagate their spend of the remote commit tx anchor.

You're really describing a form of MITM attack. If the target learns about the existence of the other commitment transaction, be it a revoked transaction or the remote's version
...

💬 kristapsk commented on pull request "Intro: Have user choose assumevalid":
(https://github.com/bitcoin-core/gui/pull/149#issuecomment-1874729869)
Concept ACK

💬 petertodd commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1874749479)
> > You've misunderstood how Lightning works. While simplified explanations often talk about [...]
>
> I see how a superficial understanding of Lightning and pinning issues could lead you to think that. However your hot fix to your proposal does not patch all its flaws.

It's not a "hot fix", it's the only way it could have ever worked. The remote and local transactions are fundamental to Lightning; I didn't mention that explicitly because I thought it would be obvious to anyone familiar wi
...

💬 zhaocaipeng commented on issue "is a getBlockTemplate transactions's bug?":
(https://github.com/bitcoin/bitcoin/issues/29166#issuecomment-1874774863)
> If you have a general question, consider asking on [stack exchange](https://bitcoin.stackexchange.com/).
>
> If this is a bug report, what was the JSON result returned from `getblocktemplate` when this happened?

I'm not entirely sure if this is a bug. Is the sum of sigops in the transaction details returned by getBlockTemplate not allowed to exceed 2000?

✅ zhaocaipeng closed an issue: "is a getBlockTemplate transactions's bug?"
(https://github.com/bitcoin/bitcoin/issues/29166)

💬 aureleoules commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1440077502)
Seems like a hazard
```suggestion
} else if (!is_trusted && txo.GetWalletTx().InMempool()) {
```

💬 aureleoules commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1440086373)
nit: Could use `contains` (and other places too)
```suggestion
if (!tx_safe_cache.contains(outpoint.hash)) {
```

💬 darosior commented on pull request "Wallet: don't underestimate the fees when spending a Taproot output":
(https://github.com/bitcoin/bitcoin/pull/26573#discussion_r1440150443)
Done.