Skip to content

refactor: renaming Ed25519 to Edwards25519#2409

Open
SimonRastikian wants to merge 27 commits intomainfrom
simon/changing-ed25519-to-Curve25519
Open

refactor: renaming Ed25519 to Edwards25519#2409
SimonRastikian wants to merge 27 commits intomainfrom
simon/changing-ed25519-to-Curve25519

Conversation

@SimonRastikian
Copy link
Copy Markdown
Contributor

Follow up to #2396.

Partly Closes #2322 namely the section related to renaming Ed25519 into the real curve name Curve25519.

@SimonRastikian SimonRastikian changed the title Renaming ed25519 to Curve25519 refactor: renaming ed25519 to Curve25519 Mar 11, 2026
@claude
Copy link
Copy Markdown

claude bot commented Mar 11, 2026

PR title type suggestion: This PR renames a curve variant across the codebase without changing functionality, which is a refactoring task. The title should include a conventional commit type prefix.

Suggested title: refactor: rename Ed25519 to Curve25519

@claude
Copy link
Copy Markdown

claude bot commented Mar 11, 2026

Code Review: refactor: renaming ed25519 to Curve25519

Straightforward rename of the internal Curve::Ed25519 variant to Curve::Curve25519 across the codebase. The change is mechanical and consistent.

Compatibility Analysis

  • Borsh (on-chain state): Safe. Borsh uses variant indices, not names. The variant remains at position 1.
  • JSON deserialization: Safe. #[serde(alias = "Ed25519")] preserves backward compatibility for callers passing "Ed25519" to vote_add_domains.
  • JSON serialization: The internal Curve enum will now serialize as "Curve25519" instead of "Ed25519". This does NOT affect external consumers because the public API view functions go through the DTO mapping (Curve::Curve25519 => SignatureScheme::Ed25519 at crates/contract/src/dto_mapping.rs:518).
  • Node code: The Curve enum is only used in-memory for match arms/logic within the node — no cross-node serialization of this type.

Minor observation (non-blocking)

The existing test_serialization_format test at crates/contract/src/primitives/domain.rs:409 only tests Curve::Secp256k1. Adding a case for Curve::Curve25519 would document the new serialization output ("Curve25519") and catch any future regressions. This is optional since the DTO layer is the actual external contract.

No critical issues found.

✅ Approved

@SimonRastikian SimonRastikian changed the title refactor: renaming ed25519 to Curve25519 refactor: renaming ed25519 to Edwards25519 Mar 11, 2026
@SimonRastikian SimonRastikian changed the title refactor: renaming ed25519 to Edwards25519 refactor: renaming Ed25519 to Edwards25519 Mar 11, 2026
@SimonRastikian SimonRastikian self-assigned this Mar 19, 2026
netrome
netrome previously approved these changes Mar 20, 2026
Copy link
Copy Markdown
Collaborator

@netrome netrome left a comment

Choose a reason for hiding this comment

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

Sorry I thought I approved this already. Nice stuff!

@netrome
Copy link
Copy Markdown
Collaborator

netrome commented Mar 20, 2026

Please rebase/merge latest main and I'll re-approve

@SimonRastikian SimonRastikian force-pushed the simon/SignatureScheme-Curve branch from 23e7d5c to d65d7ae Compare March 20, 2026 13:24
Base automatically changed from simon/SignatureScheme-Curve to main March 23, 2026 20:38
@netrome netrome dismissed their stale review March 23, 2026 20:38

The base branch was changed.

@barakeinav1 barakeinav1 self-requested a review March 24, 2026 17:44
1,
"Ed25519",
"Curve__Ed25519",
"Edwards25519",
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.

is this an external interface change?
what is the impact of it?

fields: Empty,
},
"Curve__Ed25519": Struct {
"Curve__Edwards25519": Struct {
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.

same question as above

Copy link
Copy Markdown
Contributor

@barakeinav1 barakeinav1 left a comment

Choose a reason for hiding this comment

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

I see you updated the contract snap, does this mean those changes have a interface breaking change?
and if yes, what is the impact of it?

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.

3 participants