0xPolygonHermez / cdk-erigon

Ethereum implementation on the efficiency frontier
GNU Lesser General Public License v3.0
35 stars 39 forks source link

state root missmatch on modexp edge case (fork 13) #1467

Closed xavier-romero closed 3 days ago

xavier-romero commented 1 week ago

This is the minimal contract that I've been able to create to show the issue.

contract PreModExpBUG {
    function modexp_test() public {
        assembly {
            // free memory pointer
            let memPtr := mload(0x40)
            let live_memPtr := memPtr

            // length of base
            mstore(live_memPtr, 64)
            live_memPtr := add(live_memPtr, 0x20)
            // length of exponent
            mstore(live_memPtr, 32)
            live_memPtr := add(live_memPtr, 0x20)
            // length of modulus
            mstore(live_memPtr, 32)
            live_memPtr := add(live_memPtr, 0x20)

            // call the precompiled contract BigModExp (0x05)
            // gas, address=0x5, value=0, argsOffset=memptr, argsSize=192/0xc0, retOffset=memptr, retSize=32
            let success := call(gas(), 0x05, 0x0, memPtr, 192, memPtr, 32)
            let result := mload(memPtr)
            sstore(0x1, result)  // set result
        }
    }
}

The issue raises when storing the result, because it's different from what is returned by Executor though.

xavier-romero commented 1 week ago

The issue raises when storing the result, because it's different from what is returned by Executor though. Looking at current source code:

    var (
        baseLen = new(big.Int).SetBytes(getData(input, 0, 32)).Uint64()
        expLen  = new(big.Int).SetBytes(getData(input, 32, 32)).Uint64()
        modLen  = new(big.Int).SetBytes(getData(input, 64, 32)).Uint64()
    )
    if len(input) > 96 {
        input = input[96:]
    } else {
        input = input[:0]
    }

If the input for modexp is missing the params 4, 5 and 6 (base, exp and mod) then the first 3 params (baseLen, expLen and modLen) are resused as if they also were base, exp and mod, generating a whole different result. I managed to successfully get through the test with the modifications in this PR: https://github.com/0xPolygonHermez/cdk-erigon/pull/1468 Since I'm a total GO n00b and my understanding of the whole thing is very limited, it could be a totally wrong approach, get it just as a PoC on how it's fixed.

Sharonbc01 commented 1 week ago

Another change required.

Sharonbc01 commented 3 days ago

Confirmed with @xavier-romero this issue is resolved.

Another Modex issue was found and is captured in issue #1484