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

fix: UniswapV2 test for EVM #54

Conversation

gianbelinche
Copy link
Contributor

@gianbelinche gianbelinche commented Aug 19, 2024

What ❔

This PR fixes the uniswap router v2 test adapted for EVM, the only change made was changing the order of the token's deployment:

On tests/solidity/complex/defi/UniswapV2Router01/test_evm.json
From

{
            "instance": "Token2",
            "method": "#deployer",
            "calldata": [
                "0x40",
                "0x80",
                "3",
                "0x4441490000000000000000000000000000000000000000000000000000000000",
                "14",
                "0x44616920537461626c65636f696e000000000000000000000000000000000000"
            ],
            "expected": [
                "Token2.address"
            ]
        },
        {
            "instance": "Token1",
            "method": "#deployer",
            "calldata": [
                "0x40",
                "0x80",
                "4",
                "0x5742544300000000000000000000000000000000000000000000000000000000",
                "14",
                "0x5772617070656420425443000000000000000000000000000000000000000000"
            ],
            "expected": [
                "Token1.address"
            ]
        },

To

{
            "instance": "Token1",
            "method": "#deployer",
            "calldata": [
                "0x40",
                "0x80",
                "3",
                "0x4441490000000000000000000000000000000000000000000000000000000000",
                "14",
                "0x44616920537461626c65636f696e000000000000000000000000000000000000"
            ],
            "expected": [
                "Token1.address"
            ]
        },
        {
            "instance": "Token2",
            "method": "#deployer",
            "calldata": [
                "0x40",
                "0x80",
                "4",
                "0x5742544300000000000000000000000000000000000000000000000000000000",
                "14",
                "0x5772617070656420425443000000000000000000000000000000000000000000"
            ],
            "expected": [
                "Token2.address"
            ]
        },

This new added test should only be run with EVM target, since on ERAVM target it fails, the other way around is also true for the previous existing test

Why ❔

Checklist

  • PR title corresponds to the body of PR.
  • Documentation comments have been added / updated.

Copy link
Collaborator

@hedgar2017 hedgar2017 left a comment

Choose a reason for hiding this comment

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

@gianbelinche thanks a lot for the fix!
Could you try simply adding another JSON file, e.g. test_eravm.json and test_evm.json?
Please let me know if it doesn't work.

@hedgar2017 hedgar2017 changed the title Fix uniswap test for evm fix: UniswapV2 test for EVM Aug 20, 2024
@gianbelinche
Copy link
Contributor Author

@gianbelinche thanks a lot for the fix! Could you try simply adding another JSON file, e.g. test_eravm.json and test_evm.json? Please let me know if it doesn't work.

Good idea! Didn't think of that.

@hedgar2017 hedgar2017 merged commit a31bc53 into matter-labs:az-cpr-1741-add-the-target-filter-to-tests Aug 22, 2024
1 check 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.

2 participants