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

Add explicit numeric constant value for all opcodes #1129

Merged
merged 3 commits into from
Apr 21, 2012

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Apr 21, 2012

  • Easier for debugging (what opcode was 0x... again?)
  • Clarifies that the opcodes are set in stone in the protocol, and signals that it is impossible to insert opcodes in between.

I've checked that the old and new representation are exactly the same, see http://pastebin.com/dnsyyfFD for testcase. This needs careful scrutiny though.

Edit: also added commit to change type of CMerkleTx::fMerkleVerified (and other in-memory-only flags) from char to bool. It is always (only 3 occurences) used as bool, so this is better.

Edit.2: also removed some GUI hints in the bitcoin core, which were remnants from Wx.

- Easier for debugging (what opcode was 0x... again?)
- Clarifies that the opcodes are set in stone in the protocol, and signals that it is impossible to insert opcodes in between.
@jgarzik
Copy link
Contributor

jgarzik commented Apr 21, 2012

although the pastebin test seems accurate, I would have preferred one that directly includes both old and new enum{} lists... add prefix OLD_ to each symbol in old enum, and then script-create a test that tests OLD_FOO==FOO, for each symbol.

ACK on the concept

@laanwj
Copy link
Member Author

laanwj commented Apr 21, 2012

Done that: http://pastebin.com/GtjjXRZM

I've also diffed the disassemblies (with only commit 7be8b2f): http://pastebin.com/fTvGwW94 (difference in version.o is explained by different git commit, apart from that there are no differences)

The Qt UI has its own associated structures for temporary transaction state / cache.
@jgarzik
Copy link
Contributor

jgarzik commented Apr 21, 2012

ACK

2 similar comments
@sipa
Copy link
Member

sipa commented Apr 21, 2012

ACK

@gavinandresen
Copy link
Contributor

ACK

sipa added a commit that referenced this pull request Apr 21, 2012
Add explicit numeric constant value for all opcodes
@sipa sipa merged commit d5eb41f into bitcoin:master Apr 21, 2012
coblee pushed a commit to litecoin-project/litecoin that referenced this pull request Jul 17, 2012
Add explicit numeric constant value for all opcodes
@laanwj laanwj deleted the 2012_04_opcodes branch April 9, 2014 14:14
suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 22, 2019
Fix clang compiler warning for messageTextLabel
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Dec 25, 2019
…IVX v4.0 RC

fc21c29 [Consensus] Define TestNet changeover block for PIVX v4.0 (random-zebra)

Pull request description:

  The changeover block height for new P2P message signatures, new time protocol and v7 blocks is still set at placeholder values (mainnet: `2967000`, testnet: `2214000`).

  These need to be changed to the actual values to use.

  EDIT: This PR will change only the testnet height. Mainnet will be fixed in a subsequent PR.

ACKs for top commit:
  Fuzzbawls:
    ACK fc21c29
  Warrows:
    utACK fc21c29

Tree-SHA512: c93e837f494f74b2a25186b396c9ad17943c4f94831e4c4c081b2a6794d641827003fdc2d3668aacce998293769dd6ba2b36ba790ebaa05dcec54f2317c6eb77
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants