Skip to content

Add marshalling for deal#610

Merged
thehoul merged 12 commits into
remove-protobuf-dependenciesfrom
add-marshalling-for-deal
Feb 20, 2026
Merged

Add marshalling for deal#610
thehoul merged 12 commits into
remove-protobuf-dependenciesfrom
add-marshalling-for-deal

Conversation

@thehoul

@thehoul thehoul commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

Folllow up to #609. This PR adds methods to marshal and unmarshal Deal in Pedersen and Rabin VSS compatible with V3 to ensure backward compatibility.

@thehoul thehoul self-assigned this Feb 3, 2026
@github-actions

github-actions Bot commented Feb 3, 2026

Copy link
Copy Markdown

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

@thehoul

thehoul commented Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

We discussed on Monday putting the marshaling in the internal code to prevent misuse but it's not possible because it causes a circular import (VSS will need to import internal to have the marshaling methods and internal need the structs from VSS to write the marshaling methods).
So for now, they are in the respective packages (i.e. vss.pedersen and vss.rabin) which I don't think is a bad thing: marhsaling is needed for the logic of the algorithm, not just for compatibility with V3.

Note that to marshal the Deal struct, we also need to marshal a share.PriShare struct which I was able to put in internal without causing a circular import.

@thehoul
thehoul force-pushed the remove-protobuf-dependencies branch from 1fdaf5f to 562ac70 Compare February 5, 2026 10:52
@thehoul
thehoul force-pushed the add-marshalling-for-deal branch from e6e06c5 to b049d3f Compare February 5, 2026 10:53
@github-actions

github-actions Bot commented Feb 5, 2026

Copy link
Copy Markdown

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

@thehoul thehoul added the safe PR label Feb 5, 2026

@AnomalRoil AnomalRoil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a few changes

Comment thread internal/v3marshaling.go Outdated
Comment thread internal/v3marshaling.go Outdated
Comment thread internal/v3marshaling.go
Comment thread share/vss/rabin/vss.go Outdated
Comment thread share/vss/rabin/vss.go Outdated
@thehoul
thehoul force-pushed the add-marshalling-for-deal branch from 7265864 to cbbfc93 Compare February 6, 2026 17:44
@thehoul
thehoul marked this pull request as ready for review February 10, 2026 09:54

@jbsv jbsv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor stuff, thanks a lot for all that.

Comment thread share/vss/rabin/vss.go
Comment thread share/vss/rabin/vss.go
Comment thread share/vss/pedersen/vss.go
Comment thread share/vss/pedersen/vss.go
Comment thread internal/v3marshaling.go
Comment thread internal/v3marshaling.go
Comment thread internal/v3marshaling.go
@sonarqubecloud

Copy link
Copy Markdown

@thehoul
thehoul merged commit 6bad2b3 into remove-protobuf-dependencies Feb 20, 2026
24 of 25 checks passed
@thehoul
thehoul deleted the add-marshalling-for-deal branch February 20, 2026 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants