Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 Eunovo commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1487797038)
Any specific reason why the `params` length must be 9?
🚀 fanquake merged a pull request: "v3 followups"
(https://github.com/bitcoin/bitcoin/pull/29424)
💬 maflcko commented on issue "Assertion chainman().ActiveChain()[height] fails on startup on memory-constrained system":
(https://github.com/bitcoin/bitcoin/issues/29422#issuecomment-1941461947)
This happens because the program aborts after writing the coinstats index, but before writing the block index.

I tried to reproduce this by adding an abort, but it didn't work, so far:

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 0552bde665..6493e3cb05 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2652,6 +2652,7 @@ bool Chainstate::FlushStateToDisk(
// Then update all block file information (which may refer to block and undo files).

...
💬 fanquake commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1941464197)
Concept ACK
💬 maflcko commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1941469946)
On a second thought, I wonder what the target audience is. Someone aware of a test network, and wanting to use it, should be able to create a shortcut themselves? Creating the shortcut should be the easiest of their problems to get started with a test network.
💬 Sjors commented on pull request "Move log messages: tx enqueue to mempool, allocation to blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27277#issuecomment-1941470847)
Rebased and switched `LogPrintLevel(...Debug)` to `LogDebug(...)`
🚀 fanquake merged a pull request: "test, assumeutxo: Add test to ensure failure when mempool not empty"
(https://github.com/bitcoin/bitcoin/pull/29394)
📝 sdaftuar opened a pull request: "Add imbued v3 based on template-matching"
(https://github.com/bitcoin/bitcoin/pull/29427)
This attempts to imbue v3 semantics on transactions spending outputs from a transaction that looks like a LN commitment transaction.

Opening this as a draft now so that LN folks can see what this would look like, and concept ACK/NACK as appropriate. If we want to go down this route, I can add tests and comments indicating that this behavior should be deprecated at some point in the future.

See #29319 for context, and I wrote up a summary of some statistics I gathered from analyzing data f
...
💬 TheCharlatan commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1941488430)
Concept ACK
💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1941502155)
Creating a shortcut is easy, but creating an icon with a non-confusing color is not.
💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1941508665)
I would also expect not-super-tech-savy people to be testing applications on custom signets, e.g. testing a vault GUI on Inquisition. I would hope they don't use Windows of course :-)
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1487877269)
The format of params is extendable in case more fields are added in the future. It is encoded as a concatenation of (field_id, value) tuples, protobuf style. Currently there is only one field defined `pow_target_spacing`, whose field_id is 0x01 and the lendth of encoding is 8 (int64_t). So valid values of params are:

* empty string (checked in `if` block above)
* 0x01 followed by 8 bytes of pow_target_spacing (9 bytes in total)

If length is not 0 and not 9, the value can not be parsed
...
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487922236)
Added a test.
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487922283)
Taken
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487922850)
Taken the suggested text diff.
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487922960)
Updated.
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487923094)
I'm leaving this error as is for now, unless there are other examples of breaking software.

I would also rather drop `nMaxDbCache` entirely than quietly enforce it.
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487924416)
Done
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-1941603789)
Rebased, incorporated some of the text feedback above. Dropped the RAM warning from the release note, since that's already in the help text.

> Why do we have an upper limit at all?

@sipa seemed worried about users going overboard with this setting: https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1695572615
💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1487928139)
I dropped the "check RAM" sentence.