Unit tests for Evm Binding Objects

[fabrobles92]

Closes #41

Unit tests for concreteBlockContext and concreteEVM methods .

[fabrobles92]

Hello @therealbytes , Looking for early feedback, pls let me know if something is not aligned with what you expect.

Thanks in advance :D

[therealbytes]

The block context binding tests are directionally correct but need some changes:

• No need to create so many separate tests functions that all do almost the same thing. We can put everything inside the same parent function.
• If you use a utility function to create a test block context, return a block context, not a pointer to a block context that you immediately reference every time.

This should be a good start to fix both of these issues:

func TestConcreteBlockContext(t *testing.T) {
    r := require.New(t)

    var (
        block0Hash = crypto.Keccak256Hash([]byte("test0"))
        block1Hash = crypto.Keccak256Hash([]byte("test1"))
        randomHash = crypto.Keccak256Hash([]byte("random hash"))
    )

    getHash := func(blockNumber uint64) common.Hash {
        switch blockNumber {
        case 0:
            return block0Hash
        case 1:
            return block1Hash
        }
        return common.Hash{}
    }

    blockCtx := BlockContext{
        BlockNumber: big.NewInt(123),
        GetHash:     getHash,
        Time:        456,
        GasLimit:    789,
        Difficulty:  big.NewInt(123456),
        BaseFee:     big.NewInt(456123),
        Coinbase:    common.HexToAddress("0x1234567890abcdef1234567890abcdef1234567890"),
        Random:      &randomHash,
    }

    ccBlockCtx := concreteBlockContext{ctx: &blockCtx}

    t.Run("GetHash", func(t *testing.T) {
        r.Equal(block0Hash, ccBlockCtx.GetHash(0))
        r.Equal(block1Hash, ccBlockCtx.GetHash(1))
    })
    t.Run("Timestamp", func(t *testing.T) {
        r.Equal(blockCtx.Time, ccBlockCtx.Timestamp())
    })
    t.Run("BlockNumber", func(t *testing.T) {
        r.Equal(blockCtx.BlockNumber.Uint64(), ccBlockCtx.BlockNumber())
    })
    // ...
}

Also, please keep the naming consistent and don't change the capitalization of compound words like Timestamp. It should be Timestamp, not TimeStamp. Geth already has some inconsistencies here (e.g., Blockchain vs. BlockChain) and we don't want to make it worse!

[therealbytes]

Just merged #50 to fix the delegate call issue and add an extCaller interface. Rather than passing a caller address to concreteEVM, we pass a *Contract. These are the changes you need to make to your tests:

var (
    // ...
    self       = common.BytesToAddress([]byte("self"))
    caller     = common.BytesToAddress([]byte("caller"))
    contract   = NewContract(AccountRef(caller), AccountRef(self), new(uint256.Int), 10_000_000)
)

// ...

ccVmenv := concreteEVM{
    evm:      vmenv,
    contract: contract,
}

// ...

t.Run("Create", func(t *testing.T) {
    expectedContractAddr := crypto.CreateAddress(ccVmenv.contract.Address(), statedb.GetNonce(ccVmenv.contract.Address()))
    // ...
})

// ...

Once you have that working we can go over some nitpicks in variable naming. We will merge this in and I will then open another issue to repurpose these tests as Concrete Environment tests and use the new extCaller interface to make the binding tests simpler.

[therealbytes]

I made some illustrative changes to some of your tests:

package vm

import (
    "math/big"
    "testing"

    "github.com/ethereum/go-ethereum/common"
    "github.com/ethereum/go-ethereum/common/hexutil"
    "github.com/ethereum/go-ethereum/core/rawdb"
    "github.com/ethereum/go-ethereum/core/state"
    "github.com/ethereum/go-ethereum/core/types"
    "github.com/ethereum/go-ethereum/crypto"
    "github.com/ethereum/go-ethereum/params"
    "github.com/holiman/uint256"
    "github.com/stretchr/testify/require"
)

func newTestBlockContext() BlockContext {
    var (
        block0Hash = crypto.Keccak256Hash([]byte("block0"))
        block1Hash = crypto.Keccak256Hash([]byte("block1"))
        randomHash = crypto.Keccak256Hash([]byte("random"))
    )

    getHash := func(blockNumber uint64) common.Hash {
        switch blockNumber {
        case 0:
            return block0Hash
        case 1:
            return block1Hash
        }
        return common.Hash{}
    }

    blockCtx := BlockContext{
        BlockNumber: big.NewInt(0),
        GetHash:     getHash,
        Time:        50,
        GasLimit:    1000,
        Difficulty:  big.NewInt(1234),
        BaseFee:     big.NewInt(5678),
        Coinbase:    common.HexToAddress("0x1234123412341234123412341234123412341234"),
        Random:      &randomHash,
    }

    return blockCtx
}

func TestConcreteBlockContext(t *testing.T) {
    var (
        r          = require.New(t)
        blockCtx   = newTestBlockContext()
        ccBlockCtx = concreteBlockContext{ctx: &blockCtx}
    )
    t.Run("GetHash", func(t *testing.T) {
        r.Equal(blockCtx.GetHash(0), ccBlockCtx.GetHash(0))
        r.Equal(blockCtx.GetHash(1), ccBlockCtx.GetHash(1))
    })
    t.Run("Timestamp", func(t *testing.T) {
        r.Equal(blockCtx.Time, ccBlockCtx.Timestamp())
    })
    t.Run("BlockNumber", func(t *testing.T) {
        r.Equal(blockCtx.BlockNumber.Uint64(), ccBlockCtx.BlockNumber())
    })
    t.Run("GasLimit", func(t *testing.T) {
        r.Equal(blockCtx.GasLimit, ccBlockCtx.GasLimit())
    })
    t.Run("Difficulty", func(t *testing.T) {
        r.Equal(uint256.MustFromBig(blockCtx.Difficulty), ccBlockCtx.Difficulty())
    })
    t.Run("BaseFee", func(t *testing.T) {
        r.Equal(uint256.MustFromBig(blockCtx.BaseFee), ccBlockCtx.BaseFee())
    })
    t.Run("Coinbase", func(t *testing.T) {
        r.Equal(blockCtx.Coinbase, ccBlockCtx.Coinbase())
    })
    t.Run("Random", func(t *testing.T) {
        r.Equal(*blockCtx.Random, ccBlockCtx.Random())
    })
}

func TestEVMCallStatic(t *testing.T) {
    var (
        r            = require.New(t)
        statedb, _   = state.New(types.EmptyRootHash, state.NewDatabase(rawdb.NewMemoryDatabase()), nil)
        callerAddr   = common.BytesToAddress([]byte("caller"))
        selfAddr     = common.BytesToAddress([]byte("self"))
        externalAddr = common.BytesToAddress([]byte("external"))
    )

    vm := NewEVM(BlockContext{}, TxContext{}, statedb, params.TestChainConfig, Config{})
    contract := NewContract(AccountRef(callerAddr), AccountRef(selfAddr), new(uint256.Int), 10_000_000)
    ccVm := concreteEVM{evm: vm, contract: contract}

    gas := uint64(10_000)
    slot := common.BytesToHash([]byte("key"))
    value := common.BytesToHash([]byte("value"))

    // Bytecode to load the passed slot address from storage and return it
    statedb.SetCode(externalAddr, hexutil.MustDecode("0x6000355460005260206000F3"))
    statedb.SetState(externalAddr, slot, value)

    ret, remainingGas, err := ccVm.CallStatic(externalAddr, slot.Bytes(), gas)

    r.NoError(err)
    r.Equal(value.Bytes(), ret)
    r.Less(remainingGas, gas)
}

func TestEVMCallDelegate(t *testing.T) {
    var (
        r            = require.New(t)
        statedb, _   = state.New(types.EmptyRootHash, state.NewDatabase(rawdb.NewMemoryDatabase()), nil)
        callerAddr   = common.BytesToAddress([]byte("caller"))
        selfAddr     = common.BytesToAddress([]byte("self"))
        externalAddr = common.BytesToAddress([]byte("external"))
    )

    vm := NewEVM(BlockContext{}, TxContext{}, statedb, params.TestChainConfig, Config{})
    contract := NewContract(AccountRef(callerAddr), AccountRef(selfAddr), new(uint256.Int), 10_000_000)
    ccVm := concreteEVM{evm: vm, contract: contract}

    gas := uint64(10_000)
    slot := common.BytesToHash([]byte("key"))
    value := common.BytesToHash([]byte("value"))

    // Bytecode to load the passed slot address from storage and return it
    statedb.SetCode(externalAddr, hexutil.MustDecode("0x6000355460005260206000F3"))
    statedb.SetState(selfAddr, slot, value)

    ret, remainingGas, err := ccVm.CallDelegate(externalAddr, slot.Bytes(), gas)

    r.NoError(err)
    r.Equal(value.Bytes(), ret)
    r.Less(remainingGas, gas)
}

Pay close attention to the changes I have made an incorporate them in the remaining tests. Some things to keep in mind:

• Keep things readable, maintainable, and simple. In that order.
• Use accurate, consistent, clear, descriptive names.
• Don't repeat yourself more than necessary (e.g., if your are checking a value equals the expected, and the expected is explicitly set to a non-nil value, there is no need to check the value is not nil as well).
• Test what you are testing, don't test what you are not testing.

If some of the changes I have made don't make sense to you or you think they are a bad idea, please let me know!