[Peras 18] Implement generic weighted Fait-Accompli with Local Sortition#1893
[Peras 18] Implement generic weighted Fait-Accompli with Local Sortition#1893agustinmista wants to merge 10 commits intomainfrom
Conversation
fdbd4da to
96b1d18
Compare
daa1fdd to
5e6723d
Compare
94579d3 to
21c11dc
Compare
4545066 to
b621259
Compare
92a378e to
2374824
Compare
f757764 to
466680c
Compare
466680c to
36d9a2f
Compare
7f18933 to
8fb6ada
Compare
This commit implements an initial version of the weigthed Fait-Accompli with Local Sortition committee selection scheme. This implementation is generic w.r.t. both the election and vote types, as well as the crypto scheme used by the client. Co-authored-by: Nicolas BACQUEY <nicolas.bacquey@tweag.io> Co-authored-by: Thomas BAGREL <thomas.bagrel@tweag.io> Co-authored-by: Agustin Mista <agustin.mista@moduscreate.com>
This commit defines an initial instantiation of the weighted Fait-Accompli with Local Sortition scheme for Peras using pre-existing BLS crypto. Co-authored-by: Nicolas BACQUEY <nicolas.bacquey@tweag.io> Co-authored-by: Thomas BAGREL <thomas.bagrel@tweag.io> Co-authored-by: Agustin Mista <agustin.mista@moduscreate.com>
This commit refactors the weighted Fait-Accompli conformance test suite into a composite unit test that accepts a function to preprocess the stake distribution into a value that gets shared accross individual tests. This is useful to instantiate this test suite against the real implementation, which relies on a precomputed stake distribution array that can be shared across all 3000 individual test steps. Co-authored-by: Nicolas BACQUEY <nicolas.bacquey@tweag.io> Co-authored-by: Thomas BAGREL <thomas.bagrel@tweag.io> Co-authored-by: Agustin Mista <agustin.mista@moduscreate.com>
Co-authored-by: Nicolas BACQUEY <nicolas.bacquey@tweag.io> Co-authored-by: Thomas BAGREL <thomas.bagrel@tweag.io> Co-authored-by: Agustin Mista <agustin.mista@moduscreate.com>
8fb6ada to
22e70f4
Compare
perturbing
left a comment
There was a problem hiding this comment.
Functionally, it looks good 👍
Some small details and remarks that we have to crystallize.
| -- * total non-persistent stake | ||
| -- | ||
| -- TODO: this can be more efficient if we compute the values directly on the array. | ||
| weightedFaitAccompliSplitSeats :: |
There was a problem hiding this comment.
I agree with @tbagrel1 that we should at least prevent in some way this request (committee size larger than the number of pools that have non-zero stake).
| expectedSeats = | ||
| fromMaybe 0 $ | ||
| taylorExpCmpFirstNonLower | ||
| 3 |
There was a problem hiding this comment.
This value should be scrutinized. That is, we need to understand why this was set at 3
There was a problem hiding this comment.
Added a TODO here as well.
|
|
||
| -- | Persistent committee size | ||
| newtype PersistentCommitteeSize = PersistentCommitteeSize | ||
| { unPersistentCommitteeSize :: Word64 |
There was a problem hiding this comment.
I think Word64 is the de-facto type for natural numbers in the codebase. At least I haven't come across any arbitrary-length Natural being used for long-lived values.
There was a problem hiding this comment.
I ask because we can also use Word16 with values range from 0 to 65535.
I do not expect the committee size and other so exceed that. Feels like Word16 is overkill, but if it is the defacto choice, I yield ;)
There was a problem hiding this comment.
Oh, right. You were actually suggesting to go in the opposite direction haha.
As for Word16, I don't think it matters much unless we're sending these values over the wire and control the memory/packet layout (which we are not).
My rationale here being that GHC will probably not use the remaining 48bits on something else, and instead it will just dedicate the same amount of memory as if it were a Word64 due to memory alignment. See the "Basic Types" section of this old wiki entry.
6d6d5d1 to
412f4df
Compare
|
FYI: this PR is already large enough, so I started working on its continuation in Peras 20, which includes:
|
|
@agustinmista and @tbagrel1 , please be aware of this cardano-scaling/leios-wfa-ls-demo#4 |
dnadales
left a comment
There was a problem hiding this comment.
Commenting for now waiting for a response on:
- Underflow checking (
mkExtWFAStakeDistr) - Possibly missing
isPersistentMembercheck.
| ElectionId c -> | ||
| VoteMessage c -> | ||
| VoteSignature c -> | ||
| Either String () |
There was a problem hiding this comment.
Why did we decide to use String here instead of a custom error type?
There was a problem hiding this comment.
We made it easy to instantiate this class directly with some of the crypto primitives from cardano-crypto-class, which all seem to have Either String () as the return type for verification operations.
That said, I don't have strong opinions, and we can wrap this with a ComitteeSelectionCryptoError newtype if you prefer.
EDIT: these errors are also already wrapped by the InvalidVoteSignature variant of CommitteeSelectionError.
| verifyVote vote selection = | ||
| getVoteView @c vote $ \case | ||
| PersistentVote seatIndex electionId message sig | ||
| | seatIndexWithinBounds seatIndex selection -> do |
There was a problem hiding this comment.
Don't we also need to check isPersistentMember seatIndex selection?
There was a problem hiding this comment.
Yes, good catch!
Likewise, we also need to check in the NonPersistentVote variant that, after retrieving the seat index of someone who claims to be a non-persistent voter, that seat index does not correspond to a persistent member:
NonPersistentVote poolId electionId message vrfOutput sig
| Just seatIndex <- Map.lookup poolId (candidateSeats selection)
, not (isPersistentMember seatIndex selection) ->
...| -- number of seats granted by local sortition and their stake (normalized | ||
| -- by the total non-persistent stake) | ||
| NonPersistentCommitteeMember | ||
| (NonPersistentMemberProof numSeats _sig _vrfOutput) |
There was a problem hiding this comment.
Not sure I understand what you mean but, to make it more consistent I renamed these constructors to (Non)PersistentMembershipProof.
Hopefully that sparks joy.
| stakeDistrArray = | ||
| listArray | ||
| ( SeatIndex 0 | ||
| , SeatIndex (fromIntegral (Map.size pools) - 1) |
There was a problem hiding this comment.
Could there be a potential underflow here if Map.size pools == 0?
There was a problem hiding this comment.
Yes, I was planning on discharging this case elsewhere and use a non-empty map as input here, but we may as well do it directly while constructing this structure.
I tweaked this function to return an Either WFAError (ExtWFAStakeDistr a).
|
Thanks @dnadales for the review! I think I addressed all your feedback now (last 5 commits in this PR, to be squashed) 🤞 |

This PR implements the initial version of a generic weighted Fait-Accompli with Local Sortition (wFA^LS) committee selection scheme. To allow for both Peras and Leios to reuse the same implementation, the code is abstracted in the following ways:
ElectionIdtype family: central to committee selection, it allows the client to provide their own different election identifier. Peras will use round numbers, whereas Leios will use something isomorphic to a slot number.CryptoSupportsVoteSignature: crypto interface needed to sign/verify vote signatures using a public/private key pair.CryptoSupportsVRF: crypto interface needed to compute/validate VRF outputs using a public/private key pair. This is needed by the Local Sortition fallback scheme.CryptoSupportsWFALS: Wrapper on top ofCryptoSupportsVoteSignatureandCryptoSupportsVRFthat allows us to derive both their key pairs from a single key pair. This could be used to have potentially different crypto schemes for the vote signing and VRF evaluation routines derived from the same "composite" key pair.VoteSupportsWFALS: interface needed to extract the bits relevant to committee selection and signature verification from a client vote (i.e., a concrete Peras or Leios vote).In addition, this PR also provides an initial instantiation of all these type classes for Peras using BLS crypto (the best approximation for the final one we have for now).
Other notes
ElectionId, andCryptoSupportsVoteSigning) for other committee selection schemes (e.g. wFA^IID).main.O.C.Committee.AcrossEpochsdefines a small wrapper to keep the committee selections from both the current and previous epochs at hand. This is needed to validate votes cast at the end of the previous epoch arriving at the beginning of the current one.ExtCumulativeStakeDistrdata type. This would allow both Peras and Leios to share part of the heavy work needed at the beginning of every new epoch.