💬 theuni commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#issuecomment-3358431806)
I don't understand why this is necessary. Why not use `sendmsgtopeer` if you're just sending dumb bytes that don't affect our state?

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#issuecomment-3358456391)
> I don't understand why this is necessary. Why not use `sendmsgtopeer` if you're just sending dumb bytes that don't affect our state?

We don't want to send invalid txs or txs that we would not broadcast,. We want to validates the txs before sending it to a peer.
Additionally, `peer_id` argument could take an array of ids and send the tx to multiple peers. (this is not implemented, but shouldn't be difficult to)

💬 VegArchie commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3358462415)
@Dorex45
I want progress for Bitcoin as well. I don't think these OPRETURN changes are the best way to do it. If this is the only solution, then I feel the risks don't outweigh the rewards.

That being said, wouldn't it be worth sacrificing a little more time for Bitcoin's scaling progress, in order to look into alternate methods a little more? Isn't it worth quelling the out-of-control, online 'debates,' in the name of Bitcoin security?

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2396179351)
Having some partial sigs and not all pubnonces should be a contradiction. It is not possible to create a valid partial sig without all of the pubnonces. I think in that situation it is safer to do nothing rather than try to continue by adding a new pubnonce.

💬 marcofleon commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2396228628)
Shouldn't `*block` be passed in here instead of `header`? Right now, I think the block just ends up with the random nonce from when the header was created. Although this still works half the time with the simplifed PoW check. Also, calling `GetHash()` on the header includes the merkle root, so finalizing the header should be after we calculate the merkle root below.

💬 Geremia commented on issue "No way to clear command history in RPC console or reset the console without restarting the node":
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3358599699)
Restarting `bitcoin-qt` doesn't clear the command history for me.

💬 ajtowns commented on issue "[29.x] guix build failure on ppc with xcb":
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3358617644)
Running guix-build from a mostly clean build directly three times gives me these logs, if that helps:

https://gist.github.com/ajtowns/e670603ba85bb7720cea8fc5e1215172

Maybe my guix profile is corrupted or otherwise wrong somehow?

💬 kingbillie95-max commented on something "":
(https://github.com/bitcoin/bitcoin/commit/5b8752198e979ea4987d8b6238f42f8ed9a38846#commitcomment-166969360)
5b8752198e979ea4987d8b6238f42f8ed9a38846

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396429458)
Why was this line moved?

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396433735)
Should we also throw here if m_dirty_count is not zero?

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396423379)
The benefit of the change in this commit is unclear. I think it could just be removed to make review easier.

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396411224)
> Adds test coverage by randomly calling `EmplaceCoinInternalDANGER` in `SimulationTest` to verify the accounting remains correct.

Where do we verify that the accounting is correct? This also checks that the map does not contain this coin, so the `if (inserted) {` block will always be true and we never test the other case.

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396442654)
Why was this line changed?

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396441413)
This was touched just to remove the `\n`?

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396496517)
```
/**
* Emplace a coin into cacheCoins without performing any checks, marking
* the emplaced coin as dirty.
*
* NOT FOR GENERAL USE. Used only when loading coins from a UTXO snapshot.
* @sa ChainstateManager::PopulateAndValidateSnapshot()
*/
void EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin);
```
`EmplaceCoinInternalDANGER` is not meant to be used for an overwrite according to the docs, so to mimick the assumeutxo inserts I only ca
...

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396505624)
since we're casting a boolean to boolean, but I've reverted it

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396505667)
to unify it with the line below - not sure why you'd want to leave them different, but reverted it.

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396505698)
Sure, we can add a `Assume(m_dirty_count == 0);` like in `ReallocateCache`

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396505738)
To make sure the `m_dirty_count ` updates and the `cachedCoinsUsage` updates are always done in the same order for consistency.

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2396505776)
it's meant to avoid structures like `cache.usage() += InsertCoinsMapEntry(cache.map()`, but I don't mind reverting if you find that more readable.