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

[Improvement] Compile major protocols #8

Open
3 of 5 tasks
Brianspha opened this issue Jan 21, 2025 · 0 comments
Open
3 of 5 tasks

[Improvement] Compile major protocols #8

Brianspha opened this issue Jan 21, 2025 · 0 comments
Labels
enhancement New feature or request

Comments

@Brianspha
Copy link
Collaborator

Brianspha commented Jan 21, 2025

What existing feature needs improvement?

Testing foundry-revive integration and revive compiler behavior by compiling major protocols against revive using foundry-revive.

How should it be improved?

Test the following protocols (list is WIP):

  • Uniswap V4
    • Compiles successfully with all test files excluded
    • revive emits L2-specific warnings for L1 contracts:
      Warning: It looks like you are using '.send/transfer()' without providing
      the gas amount. Such calls will fail depending on the pubdata costs.
      Copy- Files affected: PoolManager.sol, ProtocolFees.sol
    • These warnings about pubdata costs seem inappropriate for L1-targeted code
  • Sablier V2
    • Compiler exits with error: Finished compiling with standard json with status ExitStatus(unix_wait_status(256)) need to investigate where the issue is occurring
    • Compiles but requires setting RUST_MIN_STACK=33554432 RUST_MAX_STACK=33554432 e.g. RUST_MIN_STACK=33554432 RUST_MAX_STACK=33554432 forge build --resolc-compile
    • Addittionally we get warnings such as
<userStyle>Normal</userStyle>
 Warning: Your code or one of its dependencies uses the 'extcodesize' instruction, which is       
<userStyle>Normal</userStyle>
 usually needed in the following cases:                                                           
<userStyle>Normal</userStyle>
   1. To detect whether an address belongs to a smart contract.                                   
<userStyle>Normal</userStyle>
  2. To detect whether the deploy code execution has finished.                                   
<userStyle>Normal</userStyle>
 Polkadot comes with native account abstraction support (so smart contracts are just accounts     
<userStyle>Normal</userStyle>
 coverned by code), and you should avoid differentiating between contracts and non-contract       
<userStyle>Normal</userStyle>
 addresses.                                                                                       
<userStyle>Normal</userStyle>

Which seems misleading because the PVM doesnt have AA yet

  • Standard forge created project

    • Compiles successfully
    • No notable issues
  • Standard ERC20 token from OpenZeppelin

  • Standard ERC721 token from OpenZeppelin

Why is this better?

This testing will help:

  • Identify issues with the revive compiler
  • Surface integration challenges with foundry
  • Guide improvements to compiler warnings
  • Ensure smooth protocol migrations
@Brianspha Brianspha added the enhancement New feature or request label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant