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

Sort entries in StellarSDK.xdr.ScVal.scvMap(...) #1131

Closed
leighmcculloch opened this issue Jan 16, 2025 · 3 comments · Fixed by stellar/js-stellar-base#787
Closed

Sort entries in StellarSDK.xdr.ScVal.scvMap(...) #1131

leighmcculloch opened this issue Jan 16, 2025 · 3 comments · Fixed by stellar/js-stellar-base#787

Comments

@leighmcculloch
Copy link
Member

According to this report:
https://discord.com/channels/897514728459468821/1183574303921418250/1321262080061476935

the StellarSDK.xdr.ScVal.scvMap(...) function currently does not sort the entries in the map.

The network requires them to be sorted, and as @Shaptic pointed out there are some functions that do sort the entries appropriately, but not this one it seems.

@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Jan 16, 2025
@Shaptic
Copy link
Contributor

Shaptic commented Jan 16, 2025

This is generated code and so we can't exactly control how "helpful" its behavior is. If you're coding at this level, unfortunately you've just gotta know exactly what you're doing.

Per your link, this code:

const milestonesScVal = milestones.map(milestone => {
        return StellarSDK.xdr.ScVal.scvMap([[
            new StellarSDK.xdr.ScMapEntry({
                key: StellarSDK.xdr.ScVal.scvSymbol(“description”),
                val: StellarSDK.xdr.ScVal.scvString(milestone.description)
            }),
            new StellarSDK.xdr.ScMapEntry({
                key: StellarSDK.xdr.ScVal.scvSymbol(“status”),
                val: StellarSDK.xdr.ScVal.scvString(milestone.status)
            }),
            new StellarSDK.xdr.ScMapEntry({
                key: StellarSDK.xdr.ScVal.scvSymbol(“flag”),
                val: StellarSDK.xdr.ScVal.scvBool(false)
            })
        ]);
    });

Could instead be written as

const milestonesScVal = milestones.map(milestone => {
    return nativeToScVal({
        description: milestone.description,
        status: milestone.status,
        flag: false
    }, {
        type: {
            description: [ "symbol", "string" ], 
            status: [ "symbol", "string" ],
            flag: [ "symbol" ]
        }
    })
});

And the map keys will be sorted correctly. We built helpers for a reason :D

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Jan 16, 2025

Someone might want to build with the xdr types though, as we saw reported in Discord, which is reasonable.

Could we add StellarSDK.xdr.ScVal.scvMapSorted that does the same thing but sorts the entries?

@Shaptic
Copy link
Contributor

Shaptic commented Jan 16, 2025

Not there because the ScVal structure is generated. I could inject it into xdr, though.

I'd again argue that if they are digging so deep into the codebase that they're building structures from scratch, they should understand the ordering requirement. Especially since maps aren't strongly typed, so there are plenty of non-trivial sorting cases that an algorithm can't resolve. Or just... use the helpers 😆

Anyway, I added best-effort sorting in stellar/js-stellar-base#787.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants