-
Notifications
You must be signed in to change notification settings - Fork 272
Add SWAPX spec #48
Add SWAPX spec #48
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Commented just some minor things.
specs/opcode/0x90_swapx.md
Outdated
@@ -0,0 +1,24 @@ | |||
# SWAPX op code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good if we make the names of opcode files consistent - we first had it 26BYTE.md and 50POP.md, so I suggest having 90SWAPX.md here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjust file name now
specs/opcode/0x90_swapx.md
Outdated
@@ -0,0 +1,24 @@ | |||
# SWAPX op code | |||
## Procedure | |||
swapx represents op codes of swap1....swap16. which swaps the top of the stack with the 'x+1'th last element. for exmaple, SWAP3 means swaping the top of the stack with the 4th last element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some typos like "exmaple" and capitalisation of letters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
specs/opcode/0x90_swapx.md
Outdated
|
||
## Exceptions | ||
1. gas out: remaining gas is not enough | ||
2. stack underflow: when stack length is less than swap position 'x + 1', i.e stack have 3 elements but swap4 or more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps "i.e stack have 3 elements but swap4 or more" -> "i.e stack have 3 elements but the op is swap3, swap4, ..., swap16.".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be more clear & updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.