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

Dynamic federations #642

Open
wants to merge 72 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@instagibbs
Copy link
Collaborator

commented May 31, 2019

This is a proposal implementation of something myself and @apoelstra have been working on, to enable dynamic membership in the blocksigning set, fedpeg signing set, and under the same coordination mechanism, the PAK enforcement.

At a really high level:

  1. If 4/5 of last N(what we call an epoch length) blocks signal desire for a change in the parameters of the system, they are replaced with the proposed. These changes can be proposed/driven by getnewblockhex.
  2. Once dynamic federations is active(versionbits deployment), signblockscripts can only be native segwit scripts, in other words, must be a version byte followed by the witness program. The blockheader now has a witness stack as well.
  3. The fedpegscript of last 2 epochs are both allowed as a grace period for users putting money into the system.
  4. PAK enforcement has been upgraded to consensus-enforcement once dynamic federations activates

A design document is forthcoming.

TODO: More tests, including peg-in claiming post-transition

@instagibbs instagibbs requested a review from stevenroose May 31, 2019

@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 5, 2019

Proposed changes:

  1. Tighten up extension_space for pak-enforcing nodes so that proposals must only have proper key entries
  2. Forbid P2SH proposals for signblockscript
  3. Forbid OP_DEPTH for proposals for fedpegscript (we want to forbid our hacky liquidv1 script which tweaks some keys but not all, and OP_DEPTH is not in miniscript)

@instagibbs instagibbs changed the title Dynamic federations [WIP] Dynamic federations Jun 6, 2019

@instagibbs instagibbs force-pushed the instagibbs:dyna_fed_squashed branch from 7c0b5ff to 7c7c4cd Jun 6, 2019

@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019

rebased on master

@apoelstra

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

14:33:26 bash@tate-module ~$ elements-cli -chain=elementsregtest getblockheader 13f6c6e9c224792f09fc9021cf9228f4fb3e97a26e6a4ba4e500ea8fc1928459
{
  "hash": "13f6c6e9c224792f09fc9021cf9228f4fb3e97a26e6a4ba4e500ea8fc1928459",
  "confirmations": 1,
  "height": 1,
  "version": 536870912,
  "versionHex": "20000000",
  "merkleroot": "43732c47c526dfdb57203e66c2ebf9c0bff23189737b6ef432bd5040b5a697a2",
  "time": 1559831118,
  "mediantime": 1559831118,
  "signblock_witness_asm": "",
  "signblock_witness_hex": "",
  "dynamic_parameters": {
    "current": {
      "signblockscript": "00204ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260",
      "max_block_witness": 75,
      "fedpegscript": "",
      "extension_space": [
      ]
    },
    "proposed": {
      "signblockscript": "",
      "max_block_witness": 0,
      "fedpegscript": "",
      "extension_space": [
      ]
    }
  },
  "nTx": 1,
  "previousblockhash": "cd179c84c35f51825f20a3b91a18d45f0c53b5ceb744a5b6ef8f0babe809396f"
}

seems that fedpegscript and extension_space are empty even though getsidechaininfo and getblockchaininfo respectively show that it should be populated. Also we should rename extension_space.

@apoelstra

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

After some in-person discussion:

  1. The above output is expected because this is not a transition block, so the CPE is empty (except it has the signblockscript so that contextless validators can check the witness).
  2. For both fedpeg and signblock, we want to encode both the witness program and witness script in the CPE. For non-v0 programs the "witness program" is unconstrained, to be compatible with Taproot where there will be no such program.
@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 7, 2019

TODO:

to support taproot upgrade with fedpegscripts, we should always include the serialized scriptPubKey of the fedpegscript, which includes the version byte(right now we're implicitly understanding it as v0 segwit script). We should also include a followup vector of bytes that has no consensus meaning for now but could later be used to include taproot-related data.

Support signaling the peg-in confirmation depth during transitions, to be able to smoothly increase or decrease this value as necessary as a network matures

@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 7, 2019

fedpegscripts must match the v0 scriptPubKey's hash, if v1+, then no consensus meaning whatsoever, and all otherwise valid peg-ins are valid

Show resolved Hide resolved src/primitives/block.h
Show resolved Hide resolved src/primitives/block.h Outdated
Show resolved Hide resolved src/primitives/block.h Outdated
Show resolved Hide resolved src/primitives/block.h Outdated
Show resolved Hide resolved src/chain.h Outdated
Show resolved Hide resolved src/validation.cpp Outdated
Show resolved Hide resolved src/miner.cpp Outdated
Show resolved Hide resolved src/chainparams.cpp
Show resolved Hide resolved src/rpc/blockchain.cpp
Show resolved Hide resolved test/functional/feature_dynafed.py

@ElementsProject ElementsProject deleted a comment from stevenroose Jun 7, 2019

@stevenroose

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Did a first pass of review over everything. So utACK 7c7c4cd.

@instagibbs instagibbs force-pushed the instagibbs:dyna_fed_squashed branch from 0c7c9df to 205b42f Jun 18, 2019

@instagibbs instagibbs force-pushed the instagibbs:dyna_fed_squashed branch from 205b42f to 6dbf3bd Jun 18, 2019

@instagibbs instagibbs force-pushed the instagibbs:dyna_fed_squashed branch from 6b93f0e to e002aef Jun 19, 2019

@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 19, 2019

All feedback/improvements have been included.

I'm getting an intermittant error for feature_pak.py however:

2019-06-19T15:43:16.821000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_3ce4qu9m
2019-06-19T15:43:17.492000Z TestFramework (INFO): Test wallet PAK
2019-06-19T15:43:18.610000Z TestFramework (INFO): Test mempool enforcement of PAK peg-outs
2019-06-19T15:43:26.648000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/home/instagibbs/elements-dev/elements/test/functional/test_framework/test_framework.py", line 177, in main
    self.run_test()
  File "./test/functional/feature_pak.py", line 127, in run_test
    block = self.nodes[1].getnewblockhex(0, {"signblockscript":WSH_OP_TRUE, "max_block_witness":3, "fedpegscript":"51", "extension_space":extension_space_proposal})
  File "/home/instagibbs/elements-dev/elements/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/home/instagibbs/elements-dev/elements/test/functional/test_framework/authproxy.py", line 136, in __call__
    raise JSONRPCException(response['error'])
test_framework.authproxy.JSONRPCException: CreateNewBlock: TestBlockValidity failed: bad-pak-tx (code 16) (-1)
2019-06-19T15:43:26.700000Z TestFramework (INFO): Stopping nodes
2019-06-19T15:43:27.055000Z TestFramework (WARNING): Not cleaning up dir /tmp/bitcoin_func_test_3ce4qu9m
2019-06-19T15:43:27.055000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/bitcoin_func_test_3ce4qu9m/test_framework.log
2019-06-19T15:43:27.056000Z TestFramework (ERROR): Hint: Call /home/instagibbs/elements-dev/elements/test/functional/combine_logs.py '/tmp/bitcoin_func_test_3ce4qu9m' to consolidate all logs

I will try to determine if this is a real issue.

@instagibbs instagibbs changed the title [WIP] Dynamic federations Dynamic federations Jun 19, 2019

@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 19, 2019

ready for review

@instagibbs

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 19, 2019

Test failure was a case we weren't accounting for, general re-orgs. Put a heavy-handed solution there since we don't expect re-orgs, for minimal code changes.

@instagibbs instagibbs force-pushed the instagibbs:dyna_fed_squashed branch from dd722bf to 39837d5 Jun 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.