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

Add an xdr.scvMapSorted constructor to automagically sort map ScVals. #787

Merged
merged 5 commits into from
Jan 17, 2025

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Jan 16, 2025

Injects an scvMapSorted constructor into the xdr module so that people can created maps with sorted entries from raw ScMapEntry instances.

Closes stellar/js-stellar-sdk#1131.

Copy link

Size Change: +1.84 kB (+0.06%)

Total Size: 3.27 MB

Filename Size Change
dist/stellar-base.js 2.4 MB +1.43 kB (+0.06%)
dist/stellar-base.min.js 870 kB +406 B (+0.05%)

compressed-size-action

Comment on lines +383 to +397
// Both a and b are `ScMapEntry`s, so we need to sort by underlying key.
//
// We couldn't possibly handle every combination of keys since Soroban
// maps don't enforce consistent types, so we do a best-effort and try
// sorting by "number-like" or "string-like."
const nativeA = scValToNative(a.key());
const nativeB = scValToNative(b.key());

switch (typeof nativeA) {
case 'number':
case 'bigint':
return nativeA < nativeB ? -1 : 1;

default:
return nativeA.toString().localeCompare(nativeB.toString());
Copy link
Member

Choose a reason for hiding this comment

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

It is intriguing how... imperfect this is. Why does this use a different approach to nativeToScVal? nativeToScVal sorting logic appears to look like this:

      return xdr.ScVal.scvMap(
        Object.entries(val)
          // The Soroban runtime expects maps to have their keys in sorted
          // order, so let's do that here as part of the conversion to prevent
          // confusing error messages on execution.
          .sort(([key1], [key2]) => key1.localeCompare(key2))
          .map(([k, v]) => {
            // the type can be specified with an entry for the key and the value,
            // e.g. val = { 'hello': 1 } and opts.type = { hello: [ 'symbol',
            // 'u128' ]} or you can use `null` for the default interpretation
            const [keyType, valType] = (opts?.type ?? {})[k] ?? [null, null];
            const keyOpts = keyType ? { type: keyType } : {};
            const valOpts = valType ? { type: valType } : {};
            return new xdr.ScMapEntry({
              key: nativeToScVal(k, keyOpts),
              val: nativeToScVal(v, valOpts)
            });
          })
      );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this actually reveals an issue with the way nativeToScVal sorts, instead. It made the assumption that map keys are strings (because in JS all {}s use strings for keys), but if those are being encoded differently (via type), then this might 💥.

Of course this convenience method isn't supposed to be a catch-all for all possible ScVals, but it could be improved there for the easy cases (number-like). Both will work reliably for the common case, which is structs, and that's the primary goal here.

@Shaptic Shaptic merged commit 68b8bae into master Jan 17, 2025
9 checks passed
@Shaptic Shaptic deleted the sorted-maps branch January 17, 2025 00:10
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.

Sort entries in StellarSDK.xdr.ScVal.scvMap(...)
2 participants