Skip to content

Update pair bytecode#4

Draft
krisbitney wants to merge 1 commit intomainfrom
zenswap-bytecode
Draft

Update pair bytecode#4
krisbitney wants to merge 1 commit intomainfrom
zenswap-bytecode

Conversation

@krisbitney
Copy link

@krisbitney krisbitney commented Apr 18, 2025

It is necessary to use the right UniswapV2Pair.sol bytecode for the Router to work.

Implemented UniswapV2Router02 contract along with the UniswapV2Library to support liquidity operations and token swaps. Added unit tests to validate `pairFor` address calculations and ensure compatibility with a custom init code hash.
@krisbitney krisbitney requested a review from benefacto April 18, 2025 19:49
Copy link
Contributor

@benefacto benefacto left a comment

Choose a reason for hiding this comment

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

Not a smart contract dev but it sounds like your liquidity pool work depends on this, so approving on that basis. Deployment script will need to be updated at some point to match.

@krisbitney
Copy link
Author

krisbitney commented Apr 21, 2025

It might not be necessary. I see that you call addLiquidity successfully in your deployment script. That's super weird because I was unable to do the same in a different context. The reason turned out to be that the pair contract bytecode differed from the hardcoded value in the library contract.

The pairFor function in the library contract was not returning the same value as the pair address calculated in the factory contract because the init code hash differed.

    // calculates the CREATE2 address for a pair without making any external calls
    function pairFor(address factory, address tokenA, address tokenB) internal pure returns (address pair) {
        (address token0, address token1) = sortTokens(tokenA, tokenB);
        pair = address(uint(keccak256(abi.encodePacked(
                hex'ff',
                factory,
                keccak256(abi.encodePacked(token0, token1)),
                hex'96e8ac4277198ff8b6f785478aa9a39f403cb768dd02cbee326c3e7da348845f' // init code hash
            ))));
    }

I even have a test that verifies this. BUT the deployment script fails when I try to replace the router build artifact in your deployment script with the one added by this PR.

// old
import routerArtifact from "@uniswap/v2-periphery/build/UniswapV2Router02.json";
// new
import routerArtifact from "../artifacts/contracts/uniswap/UniswapV2Router02.sol/UniswapV2Router02.json";

If your deployment is working, that's all that matters. We can shelve this PR.

@krisbitney krisbitney marked this pull request as draft April 21, 2025 21:52
@benefacto
Copy link
Contributor

It might not be necessary. I see that you call addLiquidity successfully in your deployment script. That's super weird because I was unable to do the same in a different context. The reason turned out to be that the pair contract bytecode differed from the hardcoded value in the library contract.

The pairFor function in the library contract was not returning the same value as the pair address calculated in the factory contract because the init code hash differed.

    // calculates the CREATE2 address for a pair without making any external calls
    function pairFor(address factory, address tokenA, address tokenB) internal pure returns (address pair) {
        (address token0, address token1) = sortTokens(tokenA, tokenB);
        pair = address(uint(keccak256(abi.encodePacked(
                hex'ff',
                factory,
                keccak256(abi.encodePacked(token0, token1)),
                hex'96e8ac4277198ff8b6f785478aa9a39f403cb768dd02cbee326c3e7da348845f' // init code hash
            ))));
    }

I even have a test that verifies this. BUT the deployment script fails when I try to replace the router build artifact in your deployment script with the one added by this PR.

// old
import routerArtifact from "@uniswap/v2-periphery/build/UniswapV2Router02.json";
// new
import routerArtifact from "../artifacts/contracts/uniswap/UniswapV2Router02.sol/UniswapV2Router02.json";

If your deployment is working, that's all that matters. We can shelve this PR.

Thanks for laying that out. Yeah, if you're rebuilding the pair contract, the init code hash has to be updated but as long as we’re using the canonical Uniswap V2 artifacts from the NPM package (which we are in the deployment script), it should all line up. Glad we can shelve this for now.

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