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

uniswapV2LibraryGetAmountsOut summary #50

Merged
merged 4 commits into from
Oct 3, 2024
Merged

Conversation

mariaKt
Copy link
Contributor

@mariaKt mariaKt commented Oct 2, 2024

This PR adds summarized rules for function uniswapV2LibraryGetAmountsOut.

function uniswapV2LibraryGetAmountsOut(uint amountIn, address[] memory path) private returns (uint[] memory amounts) {

These rules are saving 644 steps.

I would like your opinion for this rule here

  // Skip environment updates when not needed.
  // These occur due to the multiple block statements, introduced by the if
  // statement that the while rewrites to.
  rule <k> restoreEnv( _:Map ) ~> .Statements ~> restoreEnv( E:Map ) => restoreEnv(E) ...</k>
       <summarize> true </summarize>
       <current-function> uniswapV2LibraryGetAmountsOut </current-function> [priority(40)]

This is the same question as in this PR, about whether it is OK to add this rule as a summarized rule.

@mariaKt mariaKt changed the title Getamountsout summary uniswapV2LibraryGetAmountsOut summary Oct 3, 2024
Copy link
Contributor

@Robertorosmaninho Robertorosmaninho left a comment

Choose a reason for hiding this comment

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

The same questions in this PR apply here, and I have another one: When using just the <current-function> is enough?

Otherwise, it looks good! The comments really help a lot to better understand it. If we can have more on the <store> cell, it would be even more helpful.

About your question on the current PR description and in this PR, it seems ok to me. Not sure if @dwightguth, knows something more that say this can be prejudicial or is wrong in some way.

@mariaKt mariaKt merged commit 893f877 into main Oct 3, 2024
1 check passed
@mariaKt mariaKt deleted the getamountsout-summary branch October 3, 2024 19:35
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.

2 participants