Skip to content

Add VoteTally, VoteCast, VotePlan decrypting documentation #836

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Mr-Leshiy
Copy link
Contributor

No description provided.

Copy link
Contributor

@zeegomo zeegomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't look at non-vote-cast stuff

/ %x02 ECRYPTED-VOTE PROOF-VOTE ; Private payload
CHOICE = U8
ECRYPTED-VOTE = SIZE-ELEMENT-8BIT *CYPHER-TEXT
CYPHER-TEXT = 2 * SIZE-ELEMENT-65BIT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the default should be the ristretto backend where each point is 32 bytes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by rsitretto backend ?
here is the represantation of the Cyphertext

/// ElGamal ciphertext. Given a message M represented by a group element, and ElGamal
/// ciphertext consists of (r * G; M + r * `PublicKey`), where r is a random `Scalar`.
pub struct Ciphertext {
    pub(crate) e1: GroupElement,
    pub(crate) e2: GroupElement,
}
...
impl Ciphertext {
    /// Size of the byte representation of `Ciphertext`.
    pub const BYTES_LEN: usize = GroupElement::BYTES_LEN * 2;

pub const BYTES_LEN: usize = GroupElement::BYTES_LEN * 2;

So it seems to be 65 bytes for each point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A group element could either be a point on the secp256k1 curve (like Bitcoin) or points on the Ristretto group for Curve25519 (it's a feature flag of chain-vote IIRC). The former has a 65 bytes encoding, the latter 32. The default should be ristretto for both speed and size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how and where could it be configured?
Maybe I missed something but I am see only a hardcoded version of GroupElement.

pub struct GroupElement(Point);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is one here as well

pub struct GroupElement(Point);
. One of those is re-exported inside chain-vote
pub(crate) use chain_crypto::ec::ristretto255::*;
.
Default is ristretto
default = ["ristretto255"]
.
You can also try with a transaction and check its size :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my god 😳😳😳So it is configured on the building level.
Will update

CYPHER-TEXT = 2 * SIZE-ELEMENT-65BIT
PROOF-VOTE = SIZE-ELEMENT-8BIT ANNOUNCEMENT CYPHER-TEXT R-RESPONSE
ANNOUNCEMENT = 3 * SIZE-ELEMENT-65BIT
R-RESPONSE = 2 * SIZE-ELEMENT-65BIT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is 3 group elements as well. Also, we might want to introduce another symbol like GROUP-ELEMENT instead of using SIZE-ELEMENT-32/65BIT, as that might change between builds and the default could be different.
In addition, the encoding of the group element might be useful as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct there are 3 items, but Scalars not group elements

; FRAGMENT Vote plan, vote cast, vote tally
; ####################

VOTE-PLAN = PLAN-CERT IOW BFT-SIGNATURE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. technically this is not a bft signature, although in practice they are equal
  2. it also contains the id of the committee member that is signing the transaction

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also contains the id of the committee member that is signing the transaction

Is it not a part of the ED25519-SIGNATURE ?
I thought it should corresponds to the following

UPDATE-PROPOSAL   = PROPOSAL-CERT IOW BFT-SIGNATURE
UPDATE-VOTE       = VOTE-CERT IOW BFT-SIGNATURE

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not, BTF-SIGNATURE is just 64 bytes of signature. Look at how this is used in

CONSENSUS-BFT = BFT-LEADER-ID BFT-SIGNATURE

We might need to fix UPDATE-{PROPOSAL, VOTE} if that's the case

@Mr-Leshiy Mr-Leshiy force-pushed the feature/npg-2548-update-decoding-doc branch 2 times, most recently from 1dda980 to 8917170 Compare August 3, 2022 09:39
@Mr-Leshiy Mr-Leshiy force-pushed the feature/npg-2548-update-decoding-doc branch from 8917170 to 31fafb4 Compare August 3, 2022 11:11
@Mr-Leshiy Mr-Leshiy force-pushed the feature/npg-2548-update-decoding-doc branch from 4686e2b to 2a9f8a9 Compare August 3, 2022 15:41
@Mr-Leshiy Mr-Leshiy requested a review from zeegomo August 4, 2022 08:57
Copy link

@davidmisiak davidmisiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I haven't tested the call manually. Also, maybe we could add some unit tests (e.g. HashBuilder)?

EDIT: Sorry, wrong tab 😅 Please ignore.

minikin added a commit to input-output-hk/catalyst-core that referenced this pull request Dec 9, 2022
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

Successfully merging this pull request may close these issues.

4 participants