Skip to content

Conversation

@dzmitry-lahoda
Copy link
Contributor

@dzmitry-lahoda dzmitry-lahoda commented Mar 9, 2025

  • wasm, display, debug are base58 encoded. so serde json to be too
  • json schema for serde
  • some hopefully proper alloc/std shuffling (as forced by serde impl)
  • because breaking, bumped +1 major version

not sure: is anybody uses serde for not json? like if anybody needs serde to array instead of base58 string? anyway, bumped major. also assuming binary can impl serde manually, while json expect convenience (especially in public apis)

@dzmitry-lahoda dzmitry-lahoda changed the title chore(pubkey): (may be) proper serde json support and json schema chore(pubkey): (may be) proper serde json support (encode as base59 string, not as array) and json schema Mar 9, 2025
@dzmitry-lahoda dzmitry-lahoda changed the title chore(pubkey): (may be) proper serde json support (encode as base59 string, not as array) and json schema chore(pubkey): (may be) proper serde json support (encode as base58 string, not as array) and json schema Mar 9, 2025
@juchiast
Copy link

because breaking, bumped +1 version

wouldn't this make it v3

@kevinheavey
Copy link
Contributor

not sure: is anybody uses serde for not json? like if anybody needs serde to array instead of base58 string?

yeah there are many places where bincode (which uses serde) is being used to serialize a pubkey

@joncinque
Copy link
Collaborator

I appreciate the PR, but to be honest, I'm not too keen on the change. This will very likely break things in unexpected ways downstream, without really giving any benefit.

We can leave the PR open to see if any important use-cases emerge, but without a strong motivation, I would not simply make this breaking change.

@joncinque joncinque added the breaking PR contains breaking changes label Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking PR contains breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants