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

DS block header serialization is nondeterministic #115

Open
rrw-zilliqa opened this issue Jan 26, 2024 · 2 comments
Open

DS block header serialization is nondeterministic #115

rrw-zilliqa opened this issue Jan 26, 2024 · 2 comments

Comments

@rrw-zilliqa
Copy link
Contributor

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
Copy link
Contributor Author

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

@rrw-zilliqa
Copy link
Contributor Author

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.

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

1 participant