feat: add blsgroup module

[daeMOn63]

This module extends over the cosmos-sdk/x/group module and allows to:

• generate individual votes from BLS accounts
• aggregate all the individual votes and submit them all at once

Most of the security properties from the initial PRs should be there, except for the vote expiration time which I couldn't figure out how to add back, now that the proposal expiration belongs to the group policy rather than the proposal in itself.

If needed, the previous group module work can be found on the https://github.com/fetchai/fetchd/tree/group_module branch.

I've added some basic test cases, mainly to ensure we have the proper setup and utilities to build more later. Ideally, we want to add more tests and validate all the security properties there.

[kitounliu]

About vote expiration time, this is mainly for preventing replaying attacks since aggregated vote cannot have a sequence number. If replacing vote is allowed, then without the timestamp, attacker could use an old vote to replace a new one.

We only need to compare the vote expiration time with the block time to see if it is still valid. We need to warn voters not to generate a new vote before their old vote expires.

Alternatively, we can have a sequence number for a group account (can only use that for aggregated vote) (won't work if voting for multiple proposals)

[kitounliu]

I saw MsgVoteCmd() and GetVoteBasicCmd() are merged together as MsgVoteCmd(). But then we lose the capability for submitting vote directly onchain (without aggregation). If someone was later for the offchain vote aggregation or their votes were thrown away by a malicious combiner, they should still be able to submit their vote directly onchain.

[daeMOn63]

I saw MsgVoteCmd() and GetVoteBasicCmd() are merged together as MsgVoteCmd(). But then we lose the capability for submitting vote directly onchain (without aggregation). If someone was later for the offchain vote aggregation or their votes were thrown away by a malicious combiner, they should still be able to submit their vote directly onchain.

Standalone votes can still be submitted with the standard group module (tx group vote): https://github.com/cosmos/cosmos-sdk/blob/5bde3686c4538ce53356af6e9fe40b34e4ce4a06/x/group/client/cli/tx.go#L681 ?

[kitounliu]

At the end of "func (k Keeper) VoteAgg", when adding the votes (line 98), I would just skip err (the err for vote already exists) of some votes since they are from different voters. A malicious voter could first vote online and then submit its vote again for aggregation to cause the whole aggregated vote to fail. The aggregated vote is expensive to process.

[kitounliu]

POP_verify is missing... When creating a group with bls members, we need to check their pops are verified. So we need to have a MsgCreateBlsGroup on server

[kitounliu]

I saw MsgVoteCmd() and GetVoteBasicCmd() are merged together as MsgVoteCmd(). But then we lose the capability for submitting vote directly onchain (without aggregation). If someone was later for the offchain vote aggregation or their votes were thrown away by a malicious combiner, they should still be able to submit their vote directly onchain.

Standalone votes can still be submitted with the standard group module (tx group vote): https://github.com/cosmos/cosmos-sdk/blob/5bde3686c4538ce53356af6e9fe40b34e4ce4a06/x/group/client/cli/tx.go#L681 ?

[kitounliu]

Sorry..I might've accidentally closed the pull request

[daeMOn63]

POP_verify is missing... When creating a group with bls members, we need to check their pops are verified. So we need to have a MsgCreateBlsGroup on server

@kitounliu: on this, I'm not sure why we need to check for POP at group creation? As I understand it, for now, creating groups with rogue accounts shouldn't hurt, and the POP for each account is currently checked when submitting an aggregated signature (https://github.com/fetchai/fetchd/blob/bls_group_module/x/blsgroup/keeper/msg_server.go#L71-L74) - but I agree it's not super obvious, I'll add an explicit if !bls12381.IsPopValid(acc) check here to improve it. Unless there's an issue I didn't see yet on checking that at group creation.

[kitounliu]

The pop checking in VoteAgg is too late. Pop should be checked at entry point at group creation. If members are voting for multiple proposals, then all the aggregated votes will fail because one voter's pop is not checked.

[kitounliu]

Redundance and explicity are good for security. In addition, we need to consider usability and operational costs. We won't be able to predict all the security vulnerabilities since other parts of code might change in the future. What we can do is to make the security part more independent and robust.

[daeMOn63]

The pop checking in VoteAgg is too late. Pop should be checked at entry point at group creation. If members are voting for multiple proposals, then all the aggregated votes will fail because one voter's pop is not checked.

I've added a new endpoint RegisterBLSGroup:

• the register one takes a group ID and saves it as a valid BLS group after having verified POP for all its members.
• VoteAgg has been updated to fail if the group ID provided is not a registered BLS group.
• Modifying the group (such as adding new members) invalidates the registration and thus makes it impossible to VoteAgg until it's registered again (checks made on the group ID/group version pair).

[kitounliu]

Thanks! Can we do the pop checking when adding new members? If the new members all have bls public keys and their pop are valid, then we can register the group automatically; otherwise we can unregister the group

[kitounliu]

I understand you want to use x/group from cosmos-sdk to minimise the maintenance. However, if this makes the bls group module difficult/non-intuitive to use, then perhaps we should reconsider

[kitounliu]

Can we have CreateBlsGroup which can first call sdk's CreateGroup to create a group and then register the group as a bls group? And similarly the bls version of all other operations such as UpdateBlsGroupMembers, UpdateBlsGroupAdmin, Vote, etc. This will keep the bls group as registered when there is some change to the group. It also makes the bls group self-contained to avoid confusion between sdk group and bls group. If someone calls a sdk group function on a bls group, the group will become unregistered.

[daeMOn63]

Yes, but I'd suggest doing this later and in other PRs. This one gives a first (unperfect) working version already big enough for a review.