Skip to content

Marshaling and Equal() fixes - #50

Open
pszal wants to merge 6 commits into
mainfrom
pszal/fix
Open

Marshaling and Equal() fixes#50
pszal wants to merge 6 commits into
mainfrom
pszal/fix

Conversation

@pszal

@pszal pszal commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

It fixes the following issues:

  • curvePoint.UnmarshalBinary accepts non-canonical encodings of point-at-infinity
  • curvePoint.Equal mutates receiver and operand in-place
  • MasterSecret.UnmarshalJSON has value receiver (+ added tests for JSON marshaling)

@pszal
pszal requested a review from a team as a code owner June 26, 2026 14:21
@github-actions

Copy link
Copy Markdown

👋 pszal, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

@pszal
pszal enabled auto-merge (squash) June 26, 2026 14:21

@kaleofduty kaleofduty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM other than one nit

if err != nil {
t.Fatalf("Decrypt: %v", err)
}
b, err := want.MarshalJSON()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Would it make more sense to define a data structure that contains a DecyptionShare, then invoke json.Marshal/json.Unmarshal on it to make sure that the json package interacts correctly with how we defined the interface functions? One typically doesn't directly call MarshalJSON, it's mediated through the json package.
Same for other json tests.

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.

2 participants