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

[0.17] Block v4 mandatory #470

Merged

Conversation

dongcarl
Copy link
Contributor

@dongcarl dongcarl commented Nov 18, 2018

By setting the BIPXX consensus fields to 0 we allow them to be enforced from genesis block by default.

This means blocks will be required to be at least version 4 by default.

Also adds tests.

Todo for later: BIP34_IMPLIES_BIP30_LIMIT right now means all chains will slow down validation at height 1983702. We should prod upstream to do something smarter, or allow this to be set as an argument.

@dongcarl dongcarl force-pushed the 2018-11-block-v5-mandatory branch from 48e6dcb to 3a8dc50 Compare November 18, 2018 04:11
@dongcarl
Copy link
Contributor Author

Made the relevant changes and rebased over #452, having some trouble making tests work. Seems like making g_signed_blocks true might fix it, but breaks other tests.

@dongcarl dongcarl force-pushed the 2018-11-block-v5-mandatory branch from 3a8dc50 to 5830808 Compare December 7, 2018 18:49
@instagibbs instagibbs changed the title [WIP, 0.17] Block v5 mandatory [WIP, 0.17] Block v4 mandatory Dec 11, 2018
@instagibbs
Copy link
Collaborator

instagibbs commented Dec 11, 2018

note that this new method of activating new blocks is an attempt to use the starting block of various rules at block 0. This will need reasonable review but seems to be the simplest way to set mandatory block versions.

@instagibbs
Copy link
Collaborator

For bitcoin_functional tests the original regtest activation heights will likely have to be put in the util.py function that creates the conf file.

@instagibbs
Copy link
Collaborator

I believe this will get you to passing tests, but we'll need a test of our own setting them to zero:

diff --git a/test/bitcoin_functional/functional/test_framework/util.py b/test/bitcoin_functional/functional/test_framework/util.py
index 9b2129c..145f8ce 100644
--- a/test/bitcoin_functional/functional/test_framework/util.py
+++ b/test/bitcoin_functional/functional/test_framework/util.py
@@ -309,6 +309,9 @@ def initialize_datadir(dirname, n, chain):
         f.write("con_connect_coinbase=0\n")
         f.write("anyonecanspendaremine=0\n")
         f.write("con_blockheightinheader=0\n")
+        f.write("con_bip34height=100000000\n")
+        f.write("con_bip65height=1351\n")
+        f.write("con_bip66height=1251\n")
         os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
         os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
     return datadir
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 7630e32..d250fd1 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -308,6 +308,9 @@ def initialize_datadir(dirname, n, chain):
         f.write("con_blocksubsidy=5000000000\n")
         f.write("con_connect_coinbase=0\n")
         f.write("anyonecanspendaremine=0\n")
+        f.write("con_bip34height=100000000\n")
+        f.write("con_bip65height=1351\n")
+        f.write("con_bip66height=1251\n")
         os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
         os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
     return datadir

@dongcarl dongcarl force-pushed the 2018-11-block-v5-mandatory branch from 5830808 to dc1d832 Compare December 17, 2018 10:46
- Reinstate Bitcoin BIP activation heights for tests
- Add tests for block v4 after genesis block
@dongcarl dongcarl force-pushed the 2018-11-block-v5-mandatory branch from dc1d832 to e04f6c7 Compare December 17, 2018 10:54
@dongcarl
Copy link
Contributor Author

@instagibbs Added tests, lmk if you want something more extensive.

@instagibbs instagibbs changed the title [WIP, 0.17] Block v4 mandatory [0.17] Block v4 mandatory Dec 18, 2018
@instagibbs
Copy link
Collaborator

unmarked WIP

@instagibbs
Copy link
Collaborator

oh baby that test 🥇 I gotta learn that mininode stuff too.

Not even a nit per se but why the CLTV test? To make sure that blocks are being made of v4?

@instagibbs
Copy link
Collaborator

ACK e04f6c7

@dongcarl
Copy link
Contributor Author

oh baby that test 1st_place_medal I gotta learn that mininode stuff too.

Not even a nit per se but why the CLTV test? To make sure that blocks are being made of v4?

Yup exactly! 😄

@instagibbs
Copy link
Collaborator

I added a description to the OP

@dongcarl
Copy link
Contributor Author

I added a description to the OP

Thank you sir!

@instagibbs instagibbs merged commit e04f6c7 into ElementsProject:elements-0.17 Dec 18, 2018
instagibbs added a commit that referenced this pull request Dec 18, 2018
e04f6c7 Activate BIPs 34, 65, 66 at block 0 (Carl Dong)
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