ferranbt / fastssz

Fast Ethereum2.0 SSZ encoder/decoder
MIT License
73 stars 44 forks source link

Incorrect HashTreeRoot of vectors #145

Closed po-bera closed 5 months ago

po-bera commented 5 months ago

Hi,

I tried sszgen with the following type:

type Vector6Container struct {
    VectorField []uint64 `ssz-size:"6"`
}

and tested it with the following code:

vec := make([]uint64, 6)
for i := range vec {
    vec[i] = uint64(i) + 1
}
container := &Vector6Container{
    VectorField: vec,
}
h, _ := container.HashTreeRoot()

I got

[32]uint8{0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}

I think something wrong here, as the value 5 and 6 are missing in the hash root.

itsdevbear commented 5 months ago

More data here:

func Test_Vector6SSZ(t *testing.T) {
    vec := make([]uint64, 6)
    for i := range vec {
        vec[i] = uint64(i) + 1
    }
    container := &mocks.Vector6Container{
        VectorField: vec,
    }

    mockVec := make([]mocks.Uint64, 6)
    for i := range mockVec {
        mockVec[i] = mocks.Uint64(vec[i])
    }
    mockContainer := &mocks.MockSingleFieldContainer[mocks.Vector[mocks.Uint64]]{
        Field: mockVec,
    }

    // Per https://simpleserialize.com/, the root of the vector [1, 2, 3, 4, 5, 6] is:
    // 0xac136edda3bdd2e949a19a945b1ac554e4b607d339a43c540c336098fff97f2b

    h1, err := container.HashTreeRoot()
    require.NoError(t, err)

    h2, err := ssz.MerkleizeContainerSSZ(mockContainer)
    require.NoError(t, err)

    require.Equal(t, h1, h2)
}
expected: [32]uint8{0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}

actual  : [32]uint8{0xac, 0x13, 0x6e, 0xdd, 0xa3, 0xbd, 0xd2, 0xe9, 0x49, 0xa1, 0x9a, 0x94, 0x5b, 0x1a, 0xc5, 0x54, 0xe4, 0xb6, 0x7, 0xd3, 0x39, 0xa4, 0x3c, 0x54, 0xc, 0x33, 0x60, 0x98, 0xff, 0xf9, 0x7f, 0x2b}
itsdevbear commented 5 months ago

From: https://github.com/ethereum/py-ssz, we received the same result:

class VectorContainer(HashableContainer):
    fields = [
        ("vec", Vector(uint64, 6)),
    ]

vec = VectorContainer.create(vec=(1, 2, 3, 4, 5, 6))
vec_hash = vec.hash_tree_root
print(vec_hash.hex() == "ac136edda3bdd2e949a19a945b1ac554e4b607d339a43c540c336098fff97f2b")
po-bera commented 5 months ago

More info: per ssz spec, there are two cases for vectors in chunk_count:

However, in the implementation, it is https://github.com/ferranbt/fastssz/blob/ea99d10bdae66ecbe7cde5264ef72c9bf6c48b2c/hasher.go#L344,

Therefore in the above example, the number of chunks is only 1, instead of 2. Can we simply change the code to count := uint64((len(input) + 31) / 32)?

ferranbt commented 5 months ago

Fixed in https://github.com/ferranbt/fastssz/pull/147.