Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map keys should be sorted alphabetically as Rust expects #141

Open
kalepail opened this issue Sep 12, 2023 · 3 comments
Open

Map keys should be sorted alphabetically as Rust expects #141

kalepail opened this issue Sep 12, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@kalepail
Copy link
Contributor

When an argument is or contains a Map Rust expects the keys to be alphabetically ordered but JS doesn't enforce that. This will cause unclear errors right now. Soroban client likely should perform a key sort behind the scenes in order to avoid this issue.

@kalepail kalepail added the bug Something isn't working label Sep 12, 2023
@Shaptic
Copy link
Contributor

Shaptic commented Oct 27, 2023

@willemneal is this something you can incorporate (or needs to be incorporated?) into ContractSpec? I can look into doing this in parallel for nativeToScVal.

@Shaptic Shaptic moved this from Backlog to Next Sprint Proposal in Platform Scrum Oct 27, 2023
@willemneal
Copy link
Member

Good question, I need to look at how the ContractSpec handle's maps, but I think it should be done there.

@leighmcculloch
Copy link
Member

leighmcculloch commented Nov 8, 2023

I'm not sure this is the responsibility of the contract spec.

The contract spec doesn't have an opinion on how the Map val is ordered. It presents the fields in the order of code such that they can be regenerated in the same order.

Also, Maps are used irrespective of a contract type, and so the JS lib and every SDK needs to be able to support sorting a maps entries so they're valid. Map keys may be numbers, symbols, strings, bytes, anything really. This is going to be pretty challenging.

@mollykarcher mollykarcher moved this from Next Sprint Proposal to Backlog in Platform Scrum Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

4 participants