Skip to content
This repository has been archived by the owner on Aug 30, 2024. It is now read-only.

fix: remove U256 workarounds #110

Merged
merged 4 commits into from
Jul 17, 2024
Merged

fix: remove U256 workarounds #110

merged 4 commits into from
Jul 17, 2024

Conversation

xrvdg
Copy link

@xrvdg xrvdg commented Jul 16, 2024

Fix: #97

Cairo native has fixed the byte order of U256 and in this PR we incorporate the fix and remove our workaround.

@xrvdg xrvdg requested a review from varex83 July 16, 2024 08:21
@xrvdg xrvdg changed the title Remove U256 workarounds fix: remove U256 workarounds Jul 16, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.62%. Comparing base (e9d149f) to head (62c4532).

Additional details and impacted files
@@               Coverage Diff               @@
##           native2.6.4     #110      +/-   ##
===============================================
- Coverage        80.63%   80.62%   -0.02%     
===============================================
  Files               77       77              
  Lines            10587    10579       -8     
  Branches         10587    10579       -8     
===============================================
- Hits              8537     8529       -8     
  Misses            1584     1584              
  Partials           466      466              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@varex83 varex83 left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -577,8 +577,8 @@ impl<'state> StarknetSyscallHandler for &mut NativeSyscallHandler<'state> {
}

Ok(U256 {
lo: u128::from(state[2]) | (u128::from(state[3]) << 64),
hi: u128::from(state[0]) | (u128::from(state[1]) << 64),
hi: u128::from(state[2]) | (u128::from(state[3]) << 64),
Copy link

Choose a reason for hiding this comment

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

nitpick: let's keep it in order lo: ..., hi:....

@xrvdg xrvdg merged commit 878fe94 into native2.6.4 Jul 17, 2024
15 of 16 checks passed
@xrvdg xrvdg deleted the xr/apply-u256-fix branch July 17, 2024 02:24
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.

Investigate struct ordening u256 in Cairo and Native/Blockifier
3 participants