gozilliqa-sdk icon indicating copy to clipboard operation
gozilliqa-sdk copied to clipboard

DS block header serialization is nondeterministic

Open rrw-zilliqa opened this issue 1 year ago • 2 comments

Describe the bug

When you call (DsBlockHeader).Serialize(), we attempt to serialize PoWDSWinners into the dswinners entry (field 10) of the dsblock protobuf. Sadly, PowDSWinners is map[string][Peer] and thus the order of that array is not deterministic.

This leads us to reconstruct incorrect header bytestreams in VerifyDsBlock() and VerifyDsBlock() will thus occasionally fail on correctly formed DsBlocks.

To Reproduce

Attempt to verify mainnet DSBlock 34704 . Observe that it works intermittently (and that the headerBytes generated change).

Expected behavior

I expected the block to consistently either verify or not verify (preferably, to verify against the correct DS committee, and to not verify against an incorrect DS committee)

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Ubuntu 23.10

rrw-zilliqa avatar Jan 26 '24 14:01 rrw-zilliqa

You can probably fix this by making sure that we insert the winners in the order specified by PowDSWinnersList[]

rrw-zilliqa avatar Jan 26 '24 14:01 rrw-zilliqa

Ah! This is in fact an old bug, which appeared in my code because it was using a frozen version of the library. However, there is another bug which manifests in the same way - pubkey comparisons in verifier.go are made by string compare. If you (for some reason) end up with a lowercase input DS committee member, or one without a leading "0x", they will not compare equal with the (uppercase) strings returned from the API, and the verifier will incorrectly compute the DSC membership.

rrw-zilliqa avatar Jan 27 '24 20:01 rrw-zilliqa