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

Seperate block height restrictions and feature activation #116

Merged
merged 3 commits into from
Jul 15, 2015

Conversation

dexX7
Copy link
Member

@dexX7 dexX7 commented Jul 6, 2015

This PR moves the helpers to get the lastest block height etc. into utilsbitcoin, and the block height restrictions, as well as the allowed input and output types into rules.

The enum BLOCKHEIGHTRESTRICTIONS was flattened to plain numbers.

From here on, I see a few options:

  • move the block restrictions into a namespace, which would allow something like mastercore::consensus::P2SH_BLOCK
  • provide public getters, such as IsEnabled_P2SH()
  • introduce seperate restrictions for each network (i.e. main, test, regtest)
  • refine and introduce a structure like Params() (see: chainparams.h, chainparams.cpp, chainparamsbase.cpp)

To go on with #104, it could basically go like this: old, already enabled features remain to have hardcoded const int block height restrictions, while the not-yet-enabled features are changed to int. There could be a function like EnableFeature_XXX(int block), which would update these values.

@zathras-crypto
Copy link

Sorry for not responding to this one sooner - I agree with your sentiments here bud and have cleaned up some local ideas to fit in with your PR here. It's a WIP but let me know what you think of zathras-crypto@fec622a.

Thanks as always mate!

@dexX7
Copy link
Member Author

dexX7 commented Jul 10, 2015

On the first glimpse it looks very clean and good. I'll give it a in-depth look and more thoughts later.

Can you please create a PR for it, so we have a place of discussion? :)

@zathras-crypto
Copy link

Sure dude, #129

@dexX7
Copy link
Member Author

dexX7 commented Jul 11, 2015

Given that #129 builds on top of this PR, should I close or merge this?

@dexX7 dexX7 merged commit 29c6bc6 into OmniLayer:omnicore-0.0.10 Jul 15, 2015
dexX7 added a commit that referenced this pull request Jul 15, 2015
29c6bc6 Use const int instead of enum for height restrictions (dexX7)
b43da5c Add first test for hardcoded block height restrictions (dexX7)
e7a4200 Break out block restrictions and chain/network utils (dexX7)
@dexX7
Copy link
Member Author

dexX7 commented Jul 15, 2015

I consider building on top of this PR as implicit ACK, thus the merge. :)

@dexX7
Copy link
Member Author

dexX7 commented Jul 15, 2015

Whoops, looks like I introduced an issue upstream:

diff --git a/src/qt/metadexdialog.cpp b/src/qt/metadexdialog.cpp
index 6838871..9ebf2c1 100755
--- a/src/qt/metadexdialog.cpp
+++ b/src/qt/metadexdialog.cpp
@@ -17,6 +17,7 @@
 #include "omnicore/parse_string.h"
 #include "omnicore/pending.h"
 #include "omnicore/sp.h"
+#include "omnicore/rules.h"
 #include "omnicore/tally.h"
 #include "omnicore/utilsbitcoin.h"

I'll wait, until my local build finished, then I'm going to push the fix.

@dexX7 dexX7 modified the milestone: 0.0.10.0 Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants