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

feat: implement 0x13 - SGT Opcode #271

Merged
merged 6 commits into from
Sep 5, 2023
Merged

Conversation

TAdev0
Copy link
Contributor

@TAdev0 TAdev0 commented Sep 4, 2023

Pull Request type

Please check the type of change your PR introduces:

  • Feature

What is the current behavior?

Resolves: #8

What is the new behavior?

Implementation of SGT EVM opcode

Does this introduce a breaking change?

  • No

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

Snapshot Comparison Report:

No changes in gas consumption.

fn test_exec_sgt() {
// https://github.com/ethereum/go-ethereum/blob/master/core/vm/testdata/testcases_sgt.json
assert_sgt(
0x0000000000000000000000000000000000000000000000000000000000000000,
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be nice to replace all 0x000...1 by 0x1 and all 0x000..00 by 0x0

fn assert_sgt(a: u256, b: u256, expected: u256) {
// Given
let mut ctx = setup_execution_context();
ctx.stack.push(a).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

why unwrap() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copied SLT style already validated in main , but didn't really understand why... should have asked before.

crates/utils/src/u256_signed_math.cairo Outdated Show resolved Hide resolved
@TAdev0
Copy link
Contributor Author

TAdev0 commented Sep 4, 2023

@enitrat @ClementWalter now it should be better !

@TAdev0
Copy link
Contributor Author

TAdev0 commented Sep 4, 2023

@enitrat everything should be ok

@ClementWalter ClementWalter added this pull request to the merge queue Sep 5, 2023
Merged via the queue into kkrt-labs:main with commit 1bce2fa Sep 5, 2023
2 checks passed
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.

feat: implement 0x13 - SGT opcode
3 participants