Skip to content

chore: use base58 encoding for public key serialization on node HTTP server.#1230

Merged
netrome merged 10 commits intomainfrom
1226-near-key-endpoint-should-return-base58-encoded-public-keys
Oct 6, 2025
Merged

chore: use base58 encoding for public key serialization on node HTTP server.#1230
netrome merged 10 commits intomainfrom
1226-near-key-endpoint-should-return-base58-encoded-public-keys

Conversation

@DSharifi
Copy link
Copy Markdown
Contributor

@DSharifi DSharifi commented Oct 5, 2025

closes #1226

@DSharifi DSharifi linked an issue Oct 5, 2025 that may be closed by this pull request
@DSharifi DSharifi changed the title chore: use NEAR style base58 encoding + ed25519 prefix on node HTTP server. chore: use base58 encoding + ed25519: prefix for public key serialization on node HTTP server. Oct 5, 2025
@DSharifi DSharifi changed the title chore: use base58 encoding + ed25519: prefix for public key serialization on node HTTP server. chore: use base58 encoding for public keys serialization on node HTTP server. Oct 5, 2025
@DSharifi DSharifi changed the title chore: use base58 encoding for public keys serialization on node HTTP server. chore: use base58 encoding for public key serialization on node HTTP server. Oct 5, 2025
@DSharifi DSharifi marked this pull request as ready for review October 5, 2025 22:12
@DSharifi DSharifi force-pushed the 1226-near-key-endpoint-should-return-base58-encoded-public-keys branch from 58c3283 to 83b9461 Compare October 5, 2025 23:31
Comment thread crates/node-types/src/http_server.rs Outdated
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.

Would be nice with some extra test coverage, but the current set is sufficient. I'll take this over (@DSharifi is off), extend the test coverage and add some more tests before merging.

.expect("base58 decode should work")
.try_into()
.expect("key must be decoded to 32 bytes");
}
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.

Would be nice to complement these tests with a fixture test to ensure we don't accidentally modify the serialized representation. Also it would be nice to check that the serialization is consistent with the near_sdk::PublicKey one.

Copy link
Copy Markdown
Collaborator

@netrome netrome Oct 6, 2025

Choose a reason for hiding this comment

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

Added the snapshot test in 99c6e8d. We don't have near_sdk in the workspace, so skipping it for now.

Edit: we do have it in the workspace. Don't know why I thought we didn't 😅 anyway not critical for now.

@netrome netrome enabled auto-merge October 6, 2025 09:33
@netrome
Copy link
Copy Markdown
Collaborator

netrome commented Oct 6, 2025

Updated and set to auto-merge.

@netrome netrome added this pull request to the merge queue Oct 6, 2025
Merged via the queue into main with commit e09771e Oct 6, 2025
10 checks passed
@netrome netrome deleted the 1226-near-key-endpoint-should-return-base58-encoded-public-keys branch October 6, 2025 10:18
Comment thread crates/node-types/src/http_server.rs
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.

NEAR key endpoint should return base58 encoded public keys

3 participants