Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ maflcko commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478816900)
> These two are used in consensus code as well,

No, they are not. Only one of them is used in psbt, which is not consensus.

One way to check if a function is used somewhere is to `=delete` it and compile.

The following diff compiles:

```diff
diff --git a/src/psbt.h b/src/psbt.h
index f0de079f7b..ec235ed2fc 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -212,7 +212,7 @@ void DeserializeMuSig2ParticipantPubkeys(Stream& s, SpanReader& skey, std::map<C
std::vector<unsigned char>
...
πŸ’¬ maflcko commented on pull request "ci: Add missing python3-dev package for riscv64":
(https://github.com/bitcoin/bitcoin/pull/33746#issuecomment-3468999171)
(force pushed for Alpine)
πŸ’¬ maflcko commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478845769)
Dropped the `blockstorage.cpp` changes
βœ… glozow closed an issue: "Remove *petertodd.net DNS seed for testnet and mainnet"
(https://github.com/bitcoin/bitcoin/issues/33736)
πŸ’¬ mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478861359)
fixed
πŸ’¬ mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478862391)
done, thanks
πŸ’¬ mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3469069615)
[ab8bc7a](https://github.com/bitcoin/bitcoin/commit/ab8bc7a1f877ad9e80a112dae6a12ff6ddbce076) to [48d1151](https://github.com/bitcoin/bitcoin/commit/48d11516da6351358080493fa95ea6252ce08525):
Rebased, addressed remaining feedback, including resetting `m_cached_finished_ibd` reset to cleanup. Moved out of draft.
πŸ‘‹ mzumsande's pull request is ready for review: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533)
πŸ’¬ maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478881137)
from the llm:


ReceivedBlockTransaction -> ReceivedBlockTransactions [the comment refers to the ReceivedBlockTransactions function; singular "ReceivedBlockTransaction" is incorrect and may confuse readers]
making no assumptions what must -> making no assumptions about what must [missing preposition "about" makes the phrase ungrammatical and harder to parse]
πŸ€” ajtowns requested changes to a pull request: "RPC/txoutproof: Support including (and verifying) proofs of wtxid"
(https://github.com/bitcoin/bitcoin/pull/32844#pullrequestreview-3400680165)
Transaction proofs are an interoperability feature, I don't think it makes much sense to implement this without also documenting it and providing test vectors via a BIP.
πŸ’¬ ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478870174)
I think this should probably be automatic, with the current "array of txids" result being stuck behind a `-deprecatedrpc` option.
πŸ’¬ ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478893275)
Being able to easily prove/verify that a tx was included in a reorged block seems fine?
πŸ’¬ ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478855640)
There is no such subclass?

I think all this logic should entirely be in a separate class though. For efficiency probably two paths:

* if the generation tx has no witness commitment; provde all (w)txids and the generation tx via the block partial merkle tree.
* if the generation tx does have a witness commitment; provide just the generation tx's merkle path back to the block header, and prove all wtxids via a partial merkle tree back to the witness commitment
πŸ’¬ ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478889737)
I would have expected this logic to be in merkleblock.cpp as a standard part of verifying a proof (eg so that it could be included in kernel), not in the RPC code.
πŸ’¬ ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478880286)
This information (height, confirmations, assumeutxo info) seems better obtained via `getblockheader` on the blockhash to me.
πŸ’¬ mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478901830)
fixed those.
βœ… ismaelsadeeq closed a pull request: "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity"
(https://github.com/bitcoin/bitcoin/pull/32395)
πŸ’¬ ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-3469165154)
Closing this due to lack of interest.
πŸ’¬ ismaelsadeeq commented on issue "Add a script for Signet or Regtest to unlock fee estimator":
(https://github.com/bitcoin/bitcoin/issues/32105#issuecomment-3469218336)
Reviewers of #32395 can also share weigh input here so we don’t keep this open unnecessarily.
What @yellowred suggested can be handled by the client that actually needs this functionality, rather than us having to write and maintain this script in our repository.

I took a pragmatic approach, but it received little to mixed support (~0 and +1), so it seems the general sentiment is that we don’t want to enable this. Based on the feedback from my previous (now closed) PR, this can be closed as w
...