Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FreshEyes] Extend signetchallenge to set target block spacing #14

Open
wants to merge 2 commits into
base: bitcoin-fresheyes-staging-master-29365
Choose a base branch
from

Conversation

adamjonas
Copy link
Owner

The author starius wrote the following PR called Extend signetchallenge to set target block spacing, issue number 29365 in bitcoin/bitcoin cloned by FreshEyes below:

Inspired by https://github.com/bitcoin/bitcoin/pull/27446, this PR implements the proposal detailed in the comment https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1516600820.

Rationale

Introduce the ability to configure a custom target time between blocks in a custom Bitcoin signet network. This enhancement enables users to create a signet that is more conducive to testing. The change enhances the flexibility of signet, rendering it more versatile for various testing scenarios. For instance, I am currently working on setting up a signet with a 30-second block time. However, this caused numerous difficulty adjustments, resulting in an inconsistent network state. Regtest isn't a viable alternative for me in this context since we prefer defaults to utilize our custom signet when configured, without impeding local regtest development.

Implementation

If the challenge format is OP_RETURN PUSHDATA<params> PUSHDATA<actual challenge>, the actual challenge from the second data push is used as the signet challenge, and the parameters from the first push are used to configure the network. Otherwise the challenge is used as is.

This change is backward-compatible, as per the old rules, such a signet challenge would always evaluate to false, rendering it unused.

The only parameter currently available is "target_spacing" (default 600 seconds). To set it, place 0x01<target_spacing as uint64_t, little endian> in the params. Empty params are also valid. If other network parameters are added in the future, they should use 0x02<option 2 value>, 0x03<option 3 value>, etc., following the protobuf style.

Two public functions were added to chainparams.h:

  • ParseWrappedSignetChallenge: Extracts signet params and signet challenge from a wrapped signet challenge.
  • ParseSignetParams: Parses <params> bytes of the first data push.

Function ReadSigNetArgs calls ParseWrappedSignetChallenge and 0x01<target_spacing as uint64_t, little endian>0 to implement the new meaning of signetchallenge.

The description of the flag 0x01<target_spacing as uint64_t, little endian>1 was updated to reflect the changes.

A new unit tests file, 0x01<target_spacing as uint64_t, little endian>2, was added, containing tests for 0x01<target_spacing as uint64_t, little endian>3 and 0x01<target_spacing as uint64_t, little endian>4.

The test 0x01<target_spacing as uint64_t, little endian>5 from the file 0x01<target_spacing as uint64_t, little endian>6 was modified to ensure proper understanding of the new logic.

In the functional test 0x01<target_spacing as uint64_t, little endian>7, the value of 0x01<target_spacing as uint64_t, little endian>8 was changed from 0x01<target_spacing as uint64_t, little endian>9 to the wrapped challenge, setting spacing to 30 seconds and having the same actual challenge 0x02<option 2 value>0.

The Signet miner was updated, introducing a new option 0x02<option 2 value>1 with a default of 600 seconds. It must be set to the value used by the network.

Example

I tested this PR against Mutinynet, a signet running on a custom fork of Bitcoin Core, implementing 30s target spacing. I successfully synchronized the blockchain using the following config:

0x02<option 2 value>20x02<option 2 value>30x02<option 2 value>4

The content of this wrapped challenge:

0x02<option 2 value>50x02<option 2 value>60x02<option 2 value>7

Inspired by bitcoin#27446, this commit
implements the proposal detailed in the comment
bitcoin#27446 (comment).

Rationale.

Introduce the ability to configure a custom target time between blocks in a
custom Bitcoin signet network. This enhancement enables users to create a signet
that is more conducive to testing. The change enhances the flexibility of signet,
rendering it more versatile for various testing scenarios. For instance, I am
currently working on setting up a signet with a 30-second block time. However,
this caused numerous difficulty adjustments, resulting in an inconsistent
network state. Regtest isn't a viable alternative for me in this context since
we prefer defaults to utilize our custom signet when configured, without
impeding local regtest development.

Implementation.

If the challenge format is "OP_RETURN PUSHDATA<params> PUSHDATA<actual
challenge>", the actual challenge from the second data push is used as the
signet challenge, and the parameters from the first push are used to configure
the network. Otherwise the challenge is used as is.

This change is backward-compatible, as per the old rules, such a signet
challenge would always evaluate to false, rendering it unused.

The only parameter currently available is "target_spacing" (default 600
seconds). To set it, place "0x01<target_spacing as uint64_t, little endian>" in
the params. Empty params are also valid. If other network parameters are added
in the future, they should use "0x02<option 2 value>", "0x03<option 3 value>",
etc., following the protobuf style.

Two public functions were added to chainparams.h:
  - ParseWrappedSignetChallenge: Extracts signet params and signet challenge
    from a wrapped signet challenge.
  - ParseSignetParams: Parses <params> bytes of the first data push.

Function ReadSigNetArgs calls ParseWrappedSignetChallenge and ParseSignetParams
to implement the new meaning of signetchallenge.

The description of the flag -signetchallenge was updated to reflect the changes.

A new unit tests file, chainparams_tests.cpp, was added, containing tests for
ParseWrappedSignetChallenge and ParseSignetParams.

The test signet_parse_tests from the file validation_tests.cpp was modified to
ensure proper understanding of the new logic.

In the functional test feature_signet.py, the value of -signetchallenge was
changed from OP_TRUE to the wrapped challenge, setting spacing to 30 seconds and
having the same actual challenge OP_TRUE.

The Signet miner was updated, introducing a new option --target-spacing with a
default of 600 seconds. It must be set to the value used by the network.

Example.

I tested this commit against Mutinynet, a signet running on a custom fork of
Bitcoin Core, implementing 30s target spacing. I successfully synchronized the
blockchain using the following config:

signet=1
[signet]
signetchallenge=6a4c09011e000000000000004c25512102f7561d208dd9ae99bf497273e16f389bdbd6c4742ddb8e6b216e64fa2928ad8f51ae
addnode=45.79.52.207:38333
dnsseed=0

The content of this wrapped challenge:

6a OP_RETURN
4c OP_PUSHDATA1
09 (length of signet params = 9)
011e00000000000000 (signet params: 0x01, pow_target_spacing=30)
4c OP_PUSHDATA1
25 (length of challenge = 37)
512102f7561d208dd9ae99bf497273e16f389bdbd6c4742ddb8e6b216e64fa2928ad8f51ae
(original Mutinynet challenge, can be found here:
https://blog.mutinywallet.com/mutinynet/ )
Copy link

An author commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1921735275 at 2024/02/01, 16:36:24 UTC.

Copy link

An author commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1922026282 at 2024/02/01, 19:02:05 UTC.

@@ -341,7 +341,7 @@ class SigNetParams : public CChainParams {
consensus.CSVHeight = 1;
consensus.SegwitHeight = 1;
consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
consensus.nPowTargetSpacing = 10 * 60;
consensus.nPowTargetSpacing = options.pow_target_spacing;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An author commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1479616697 at 2024/02/06, 11:22:01 UTC.

Copy link

@fresheyes-staging-bot fresheyes-staging-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An author reviewed and commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-1864901669 at 2024/02/06, 11:22:01 UTC.

// - 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.

if (params.size() != 1 + 8) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 authors commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1487797038 at 2024/02/13, 12:52:10 UTC
  • comment link https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1487877269 at 2024/02/13, 13:41:36 UTC
  • comment link https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1496917274 at 2024/02/21, 05:05:13 UTC
  • comment link https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1497768481 at 2024/02/21, 15:17:22 UTC.

Copy link

@fresheyes-staging-bot fresheyes-staging-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An author reviewed and commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-1877864061 at 2024/02/13, 12:52:10 UTC.

Copy link

An author commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1941488430 at 2024/02/13, 13:13:06 UTC.

Copy link

@fresheyes-staging-bot fresheyes-staging-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An author reviewed and commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-1877978078 at 2024/02/13, 13:41:36 UTC.

Copy link

@fresheyes-staging-bot fresheyes-staging-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An author reviewed and commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-1892116098 at 2024/02/21, 05:05:13 UTC.

Copy link

An author commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1955903839 at 2024/02/21, 05:05:28 UTC.

Copy link

@fresheyes-staging-bot fresheyes-staging-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An author reviewed and commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-1893493623 at 2024/02/21, 15:17:22 UTC.

Copy link

An author commented here with:

  • comment link https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1965170097 at 2024/02/26, 20:14:18 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants