Skip to content

txscript: Remove OP_SMALLDATA#954

Merged
davecgh merged 3 commits intodecred:masterfrom
dnldd:merge_removesmalldata
Dec 29, 2017
Merged

txscript: Remove OP_SMALLDATA#954
davecgh merged 3 commits intodecred:masterfrom
dnldd:merge_removesmalldata

Conversation

@dnldd
Copy link
Copy Markdown
Member

@dnldd dnldd commented Dec 23, 2017

This PR contains the following upstream commits:

  • f6cd49a
    • NOOP
  • 4494f0f
    • updated TestOpcodeDisasm OP_UNKNOWN# range checks.

Copy link
Copy Markdown
Member

@davecgh davecgh Dec 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an extra tab snuck in here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here.

Copy link
Copy Markdown
Member

@davecgh davecgh Dec 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would introduce a hard fork. It needs to remain opcodeInvalid.

In fact, I would suggest this should be named OP_INVALID249 to make this clear. It should also be kept in the section with the comment as the reason for it being reserved/invalid did not change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be sure to assert modifications to upstream commits before putting up a PR. Sorry about that.

@dnldd dnldd force-pushed the merge_removesmalldata branch 2 times, most recently from 7631065 to b300e7a Compare December 23, 2017 18:10
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be an extra tab here.

Copy link
Copy Markdown
Member

@davecgh davecgh Dec 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to do this. Instead, just replace the 0xf9 entry in the map with OP_INVALID249.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@dnldd dnldd force-pushed the merge_removesmalldata branch from b300e7a to e44f635 Compare December 29, 2017 19:43
@dnldd dnldd force-pushed the merge_removesmalldata branch from e44f635 to e582881 Compare December 29, 2017 19:57
@davecgh davecgh merged commit e582881 into decred:master Dec 29, 2017
@dnldd dnldd deleted the merge_removesmalldata branch June 4, 2018 23:38
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.

4 participants