Skip to content

Conversation

@dragan2234
Copy link

Copy link
Collaborator

@cicr99 cicr99 left a comment

Choose a reason for hiding this comment

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

Could you also add the modified Solidity contract?

@dragan2234 dragan2234 requested a review from cicr99 January 27, 2023 03:00
@dragan2234
Copy link
Author

@cicr99 I've just updated the PR. I realized that we could implement modular inversion in cairo instead of expmod in a very simple way. Since modInv is actually expmod(a,n-2,n). I've proposed this function to be included in cairo-libs:

starkware-libs/cairo-lang#144

Copy link
Collaborator

@rodrodros rodrodros left a comment

Choose a reason for hiding this comment

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

Awesome work!

@@ -0,0 +1,18 @@
# Transpiled [BN256G2.sol](https://github.com/musalbas/solidity-BN256G2) library.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add the warp version you used, as well as the Cairo version? Also, could it be possible to have the solidity performance info when used the same input on the solidity contract (like gas use and execution time, I believe it may be helpful for future re-transpilations with cairo 1).


A few changes needed to be made to the contract in order for warp to transpile it correctly.
- The keyword "library" needed to be changed to "contract"
- Solidity version needed to be updated to 0.8.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would specify 0.8.14 which is the solc version used by warp

Comment on lines +9 to +10
This contract doesn't work properly on starknet because expmod is too heavy for the current implementation of the sequencer in starknet. One expmod with 256 bit as exponent takes 500k steps.
That's why it is imposible to test this.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you expand on this a little bit more. Like how much of this heavy computation is Warps fault or the current sequencer.

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.

3 participants