relab / hotstuff

MIT License
166 stars 52 forks source link

What is the logic behind getting the first id in the ID map, when creating a partial certificate? #82

Closed PasinduTennage closed 1 year ago

PasinduTennage commented 1 year ago

I find the following code segment in the types.go

https://github.com/relab/hotstuff/blob/5196107d449a52e240ab4eddf7ded7cf96a755e3/types.go#L144

The following is how I interpret the code.

Go map iteration is non-deterministic, hence a random ID will be chosen as the first ID inside the RangeWhile. Then, irrespective of the ID, signer is set to that random ID, and return false (which breaks the iteration)

In my understanding, what should be done is: set the signer part of the PartialCert to self. Hence i propose the following modification.


// NewPartialCert returns a new partial certificate.

func NewPartialCert(id ID, signature QuorumSignature, blockHash Hash) PartialCert {

    return PartialCert{id, signature, blockHash}

}

Am I missing something here?

Thanks

leandernikolaus commented 1 year ago

Hi and thanks for your interest.

NewPartialCert is used not only after signing but also when unmarshaling a signature received via the network. The given segment ensures signer is set to one of the nodes whose signature is included. You are correct that the representation is a bit misleading, since signature may contain signature shares from multiple signers, and only one is set. The field is intended as a convenience, for the common case where signature only contains one share. In case you want to access all signers use cert.Signature().Participants()

PasinduTennage commented 1 year ago

Thank you for the clarification