Bitcoin Core Github
44 subscribers
122K links
Download Telegram
maflcko closed a pull request: "refactor: simplify GetAncestor"
(https://github.com/bitcoin/bitcoin/pull/31778)
💬 maflcko commented on pull request "refactor: simplify GetAncestor":
(https://github.com/bitcoin/bitcoin/pull/31778#issuecomment-3095681699)
Closing for now due to inactivity. Feel free to ask for this to be re-opened or you may open a fresh pull request, once you are working on this again.
💬 Sjors commented on issue "wallet: render BIP388 wallet policies":
(https://github.com/bitcoin/bitcoin/issues/32659#issuecomment-3095726908)
Meanwhile I started working on #33008 which has most of the functionality to send a BIP388 policy to an external signer, get a resulting hmac and store that.

But I haven't worked on converting our descriptors to BIP388 policies yet.
🤔 Sjors reviewed a pull request: "test: add test cases to wallet_signer.py"
(https://github.com/bitcoin/bitcoin/pull/33020#pullrequestreview-3037095566)
These tests have been disabled from the start in d4b0107d68a91ed4d1a5c78c8ca76251329d3f3c of #16546.

For the third commit 74fb47f071451145b550062e382416db9388433d: I suggest deleting the commented out code. It's not doing anything useful for the test, and I can't remember why I wrote it.
💬 Sjors commented on pull request "test: add test cases to wallet_signer.py":
(https://github.com/bitcoin/bitcoin/pull/33020#discussion_r2218550645)
38a6293fa89104f3d6f1907d04c2e4c488157307: this makes it more clear how `set_mock_result` is intended to be used:

```py
self.set_mock_result(self.nodes[1], '0 {"invalid json"}')
```
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3095873057)
Rebased on master@ https://github.com/bitcoin/bitcoin/commit/5878f35446
👍 dergoegge approved a pull request: "fuzz: Make process_message(s) more deterministic"
(https://github.com/bitcoin/bitcoin/pull/32822#pullrequestreview-3037221638)
reACK fa1a14a13a15ecfb7587a94ee86b4ace7c819519
💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2218671774)
IIUC The silent payments wallet needs to scan entire blocks since it doesn't know ahead of time which `scriptPubKeys` to use in a GCS filter. Since we can't skip blocks, fast rescans shouldn't provide any advantages here.
💬 stickies-v commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2218745362)
> I'm wondering if it makes more sense to signal the lack of something ... instead of the additional functionality in the pair

I considered that too, but I don't think that's better. The "Unchached" one is unsafe, and for that it should stand out. My initial thought was to name it `GetAddressesUnsafe` or `GetAddressesInternal`, but those names carry less meaning, so I eventually landed on just describing what it does, which I think at least gives people somewhat familiar with the code a good
...
📝 maflcko opened a pull request: "NOMERGE DEBUG WIP ignore"
(https://github.com/bitcoin/bitcoin/pull/33028)
💬 fanquake commented on pull request "ci: Use APT_LLVM_V in msan task":
(https://github.com/bitcoin/bitcoin/pull/32999#issuecomment-3096172593)
Should probably adjust the timeout / comment:
https://github.com/bitcoin/bitcoin/blob/5878f35446ae260ccb0ab5051b795b4364f77951/.cirrus.yml#L160
but given it's being moved in #32989, left a note there.
💬 fanquake commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2218810639)
Note to remember to drop the timeout post #32999.
🤔 ajtowns reviewed a pull request: "refactor: GenTxid type safety followups"
(https://github.com/bitcoin/bitcoin/pull/33005#pullrequestreview-3037443575)
Approach ACK
💬 ajtowns commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2218771665)
Maybe add a comment to call attention to the `const&`and the reason for it?
💬 ajtowns commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2218799503)
Passing in the `GenTxid` and keeping the `visit` inside the function (rather than an `if` outside it along with templatizing) seems nicer?
🚀 fanquake merged a pull request: "ci: Use APT_LLVM_V in msan task"
(https://github.com/bitcoin/bitcoin/pull/32999)
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2218854573)
Unrelatedly, the `USE_CIRRUS_RUNNERS` can probably be removed:

* It is only there for forks, but the comment says it doesn't work for forks anyway in the expected way.
* If someone maintains a fork (presumably to add patches), it should be trivial for them to make the required patch to the CI as well.
🚀 fanquake merged a pull request: "test: fix `ReadTopologicalSet` unsigned integer overflow"
(https://github.com/bitcoin/bitcoin/pull/33007)
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-3096237139)
Rebased after the merge of #32521 to pick up recent changes to `AreInputsStandard`.

A new verbose message will be returned when the BIP54 legacy sigops limit is violated:

* `bad-txns-non-witness-sigops-exceeds-bip54-limit`: This occurs when the total number of non-witness sigops across the entire transaction exceeds `MAX_TX_LEGACY_SIGOPS`. Additionally, a debug message is also returned with the actual number of sigops and the limit, for example: `sigops 2501 > limit 2500`.


[3cde7f1852
...
🤔 ismaelsadeeq reviewed a pull request: "mempool: Avoid needless vtx iteration during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827#pullrequestreview-3037622642)
reACK 249889bee6b88eb9814eb969e6fb108f86a4bf98