Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 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`.
💬 theuni commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2153158803)
Any reason this is still in draft? I agree it seems useful do enable before the CMake switch.
💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1630032080)
hmmm I'm struggling to get a link to survive the lint test
💬 instagibbs commented on pull request "Ephemeral Anchors, take 2":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2153175605)
going to put this into draft until https://github.com/bitcoin/bitcoin/pull/29496 is merged, since dust checks are completely off when `-acceptnonstdtxn=1`. which is required for TRUC currently.
📝 instagibbs converted_to_draft 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 (can be shared by multiple parties)
2. Single unkeyed anchor, ala P2WSH(OP_TRUE), P2SH(OP_TRUE)

Which is
...
💬 cbergqvist commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1630046789)
If the code is unable to communicate why it is like it is, it's good to explain the otherwise surprising state of it. It would hopefully limit the amount of questions like my prior one from https://github.com/bitcoin/bitcoin/pull/26596#pullrequestreview-2089208116:
> `LegacyScriptPubKeyMan::NewKeyPool()` only touches data from `LegacyDataSPKM`, should it be moved to that class as public (or protected and then exposed in the subclass?).

But I guess you're fairly confident it will be deleted
...
💬 sr-gi commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#issuecomment-2153187235)
ACK [6eecba4](https://github.com/bitcoin/bitcoin/pull/29575/commits/6eecba475efd025eb011400af58621ad5823994e)
💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2153187763)
reACK https://github.com/bitcoin/bitcoin/pull/30161/commits/ee253ca7dea9bed01d4c1800760477ef06310df8

reviewed via `git range-diff master e4ecb82 ee253ca`
💬 sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1630056430)
https://github.com/bitcoin/bitcoin/pull/29575#issuecomment-2152662655

Will rebase on top of that
💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2153199835)
reACK https://github.com/bitcoin/bitcoin/pull/30111/commits/a9a6de5a7c8c411519622a32bd4b998df5d7d883

Only relevant change is suggested commit message fix

reviewed via `git range-diff master ef8de26 a9a6de5`