Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 hebasto approved a pull request: "util: add VecDeque"
(https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2102299643)
re-ACK e4ecb8217ada3dae1c1645a8d0a12e14b0f935da, I agree with changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2088736908).
💬 hebasto commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1629750737)
This copy assignment can be amended to handle the self-assignment as well to do not conflict with https://github.com/bitcoin/bitcoin/pull/30234.
💬 hebasto commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1629747141)
Does it make sense to label branches with the `[[likely]]` and `[[unlikely]]` attributes here?
💬 murchandamus commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1629749583)
The file name could also be renamed to refer to TRUC transactions instead of v3_policy.
🤔 murchandamus reviewed a pull request: "policy: bump TX_MAX_STANDARD_VERSION to 3"
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-2102303518)
ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c
💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1629758949)
Yes, interesting edge case! I extended the comment.
💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1629759103)
Done
📝 pinheadmz opened a pull request: "json-rpc 2.0 followups: docs, tests, cli"
(https://github.com/bitcoin/bitcoin/pull/30238)
This is a follow-up to #27101.

- Addresses [post-merge comments ](https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606723428)
- bitcoin-cli uses JSON-RPC 2.0
- functional tests use JSON-RPC 2.0 by default (exceptions are in the regression tests added by #27101)
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1629801393)
I think the meaning is well understood, both before and after `LegacyScriptPubKeyMan` is deleted. `SPKM` is an abbreviation for `ScriptPubKeyMan` anyways.
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1629814874)
The line this comment is on is in the one that goes over the non-HD keys. However, I see that you're confused about the similar line in the loop below that iterates the hd chains.

The `keypool_size` here is a runtime value that depends on the startup options. It's is set when the `DescriptorScriptPubKeyman` is instantiated on loading, and the value is never persisted to disk.

The value that does matter is the `range_end` stored in `WalletDescriptor`. This is persisted to disk. When we are
...
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1629821488)
Added a comment.
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1629823159)
I don't think it's necessary to have a comment saying such.
💬 maflcko commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1629850553)
I'd say absolute links make sense in this case, because the file will be moved without modification later on into a different folder (`./doc/release-notes/`)
📝 instagibbs opened a pull request: "Ephemeral Anchors, take 2"
(https://github.com/bitcoin/bitcoin/pull/30239)
A replacement for https://github.com/bitcoin/bitcoin/pull/29001

Now that we have 1P1C relay, TRUC transactions and sibling eviction, it makes sense to retarget this feature more narrowly by not introducing a new output type for now, and simple focusing on the feature of allowing temporary dust in the mempool.

Users of this can immediately use dust outputs as:
1. Single keyed anchor
2. Single unkeyed anchor, ala P2WSH(OP_TRUE), P2SH(OP_TRUE)

Future upgrades can include:
1. Adding `OP_
...
instagibbs closed a pull request: "Ephemeral Anchors"
(https://github.com/bitcoin/bitcoin/pull/29001)
💬 instagibbs commented on pull request "Ephemeral Anchors":
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2153063105)
closing this in favor of https://github.com/bitcoin/bitcoin/pull/30239
💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1629970936)
Done.
💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1629971528)
Done, though naive self-assignment isn't actually broken here. It's just test code though, so better to be more obviously correct.
💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2153072603)
I've added some doxygen comments on `VecDeque`.
💬 sipa commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#issuecomment-2153089008)
@sr-gi Done, in a new commit, for both `MaybePunishNodeForBlock` and `MaybePunishNodeForTx`.