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

There's two types of flags: consensus and script #9271

Closed
wants to merge 10 commits into from

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Dec 3, 2016

Replaces #7779
Unlike the previous, this distinguishes between consensus flags and the subset of them that are only related to script. We can decouple them from each other.

There's extra exploration in https://github.com/jtimon/bitcoin/commits/0.13-consensus-flags
Things to explore:

  • Calling GetConsensusFlags from ContextualCheckBlock or "upwards" should look bad in benchmarks (although maybe not so much since bip9 is cached and ISM has been eliminated), but would complete GetConsensusFlags().

  • Moving GetConsensusFlags "upwards" can reduce the impact of unifying logic from both ConnectBlock and ContextualCheckBlock in GetConsensusFlags.

@reinier19
Copy link

good

@reinier1991
Copy link

hajs

@jtimon jtimon force-pushed the 0.13-consensus-flags-error branch 3 times, most recently from 12bd793 to 711d92a Compare December 5, 2016 11:59
@jtimon jtimon changed the title Discusion : 0.13 consensus flags error Discussion: There's two types of flags: consensus and script Dec 5, 2016
@jtimon
Copy link
Contributor Author

jtimon commented Dec 5, 2016

Updated code and description.

EDIT: sorry! still failing tests for a supposedly simple POC!

Also sorry for maybe making more commits than necessary, squash suggestions very wecomed, specially if they contain suggestions about the new commit message.

@jtimon jtimon force-pushed the 0.13-consensus-flags-error branch 8 times, most recently from 9b1a273 to a813b0a Compare December 20, 2016 03:50
@jtimon jtimon force-pushed the 0.13-consensus-flags-error branch from e12cd59 to 1c0a3d9 Compare April 4, 2017 20:01
@jtimon
Copy link
Contributor Author

jtimon commented Apr 4, 2017

Rebased, still don't understand why the script_tests.cpp are failing.

@jtimon jtimon changed the title Discussion: There's two types of flags: consensus and script There's two types of flags: consensus and script Apr 5, 2017
@jtimon jtimon force-pushed the 0.13-consensus-flags-error branch from 1c0a3d9 to f2fc1c6 Compare May 19, 2017 02:26
@jtimon
Copy link
Contributor Author

jtimon commented May 19, 2017

Needed rebase, rebased on top of #10427

@jtimon jtimon force-pushed the 0.13-consensus-flags-error branch from f2fc1c6 to b6a84fc Compare June 14, 2017 06:19
@jtimon
Copy link
Contributor Author

jtimon commented Jun 14, 2017

Needed rebase, even though the first commit of #10192 hasn't been merged yet (that will need rebase too).

jtimon added 10 commits July 4, 2017 20:12
...to make sure only consensus flags as an interface to bitcoinconsensus

TODO: Decouple the bits in consensus flags from the ones in the interpreter flags
this also helps test ScriptFlagsFromConsensus
- Replacing GetBlockScriptFlags
- Use GetConsensusFlags and ScriptFlagsFromConsensus from ConnectBlock
  and LimitMempoolSize
...and use it from GetConsensusFlags()

Nothing can be deployed the block after genesis

(ie BIP30: Don't look for ancestors of the genesis block)
@jtimon jtimon force-pushed the 0.13-consensus-flags-error branch from b6a84fc to 6bd6a57 Compare July 4, 2017 18:17
@jtimon
Copy link
Contributor Author

jtimon commented Jul 4, 2017

Needed rebase after #10192. I still don't understand why the tests commented in 994551a are failing.

@jtimon
Copy link
Contributor Author

jtimon commented Aug 24, 2017

Closing for now

@jtimon jtimon closed this Aug 24, 2017
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants