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

Investigate inconsistent ssz deserialize #2648

Closed
twoeths opened this issue Jun 8, 2021 · 5 comments
Closed

Investigate inconsistent ssz deserialize #2648

twoeths opened this issue Jun 8, 2021 · 5 comments
Assignees

Comments

@twoeths
Copy link
Contributor

twoeths commented Jun 8, 2021

Describe the bug

Given this block 32768 in oonoonba network
signed_block_32768.zip

const signedBlock = ssz.altair.SignedBeaconBlock.createTreeBackedFromBytes(signedBlockBuf);
console.log("@@@ loaded signed block successfully at slot", signedBlock.message.slot, "root", toHexString(ssz.altair.BeaconBlock.hashTreeRoot(signedBlock.message)));

gives us @@@ loaded signed block successfully at slot 32768 root 0xc1ad6fc70d6c83cba35288fc58441325bbb985b16b407bdff2857ba6a30fafd0 which is correct (since we can process that block)

while

const signedBlock = ssz.altair.SignedBeaconBlock.deserialize(signedBlockBuf);
console.log("@@@ loaded signed block successfully at slot", signedBlock.message.slot, "root", toHexString(ssz.altair.BeaconBlock.hashTreeRoot(signedBlock.message)));

gives us @@@ loaded signed block successfully at slot 32768 root 0x9b1482b3be4086d530feb8954f6b5a3a975381c26d2a97482f51db974b5fd59a

this is incorrect block root and we got Invalid block signature processing that block

Expected behavior

  • ssz.altair.SignedBeaconBlock.deserialize() should be the same to ssz.altair.SignedBeaconBlock.createTreeBackedFromBytes()
  • Need to investigate what caused the issue at ssz level
@twoeths
Copy link
Contributor Author

twoeths commented Jun 8, 2021

  • there is no issue with equal, it's just issue with hashTreeRoot
  • hashTreeRoot(signedBlockStruct.message.body.syncAggregate.syncCommitteeBits) turns out to be incorrect

Note that BitVectorType is a new type in altair, that's why we haven't seen the issue during phase0 sync.

@dapplion
Copy link
Contributor

dapplion commented Jun 8, 2021

  • there is no issue with equal, it's just issue with hashTreeRoot
  • hashTreeRoot(signedBlockStruct.message.body.syncAggregate.syncCommitteeBits) turns out to be incorrect

Note that BitVectorType is a new type in altair, that's why we haven't seen the issue during phase0 sync.

Should we add unit tests for that type in the ssz repo?

@twoeths
Copy link
Contributor Author

twoeths commented Jun 8, 2021

we haven't implemented ssz spec tests, that should scan these ssz issues if we test both struct type and tree backed type

@dapplion
Copy link
Contributor

dapplion commented Jun 8, 2021

we haven't implemented ssz spec tests, that should scan these ssz issues if we test both struct type and tree backed type

Should we prioritize implementing those then?

@twoeths twoeths self-assigned this Jun 8, 2021
@twoeths
Copy link
Contributor Author

twoeths commented Jun 12, 2021

Fixed in ssz 0.8.8

@twoeths twoeths closed this as completed Jun 12, 2021
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

No branches or pull requests

2 participants