Unify ScInt & scValToBigInt into XdrLargeInt

[Shaptic]

What

• XdrLargeInt has been renamed to Int
• ScInt and scValToBigInt have been removed to simplify the API surface:

// before:
new ScInt(value);
new ScInt(value, { type: 'i128' });
// after:
Int.fromValue(value);
new Int('i128', value);

// before:
scValToBigInt(scv);
// after:
Int.fromScVal(scv).toBigInt();

• Int also has new methods and features:
  • fromValue(x: number|string|BigInt) will create an Int of an appopriate size for the given value
  • fromScVal(x: xdr.ScVal) will create an Int from an ScVal
  • .toU32() will return an ScVal of the type scvU32
  • .toI32() will return an ScVal of the type scvI32
  • the constructor now accepts i32 and u32 as the first type parameter

Why

Addresses #722: we want to reduce and unify the API surface of working with big numbers and ScVals into a single object.

[github-actions]

Size Change: +4.51 kB (+0.13%)

Total Size: 3.44 MB

Filename	Size	Change
dist/stellar-base.js	2.54 MB	+3.27 kB (+0.13%)
dist/stellar-base.min.js	899 kB	+1.24 kB (+0.14%)

_{compressed-size-action}

[karthikiyer56]

lgtm

[orbitlens]

LGTM. The only question, does this implementation really requires static methods fromValue and fromScVal? I'd prefer to just use a constructor with whatever input people going to put there. You can dynamically determine the type of the constructor argument and parse it. In this case the type argument can be optional.

E.g.

constructor(value: number|string|BigInt|xdr.ScVal, type: string?) {
  switch(typeof value) {
    case 'number': ... break
    case 'string': ... break
    case 'bigint': ... break
    default: //check for ScVal with duck-typing
    break
  }
}

new Int(12, 'i128'); 
new Int('12', 'u64'); 
new Int(12n, 'u128'); 
new Int(scv); 

The only potential problem might be with ScVal if people use multiple StellarBase libs in a project (for example importing StellarBase and StellarSDK, I faced this problem multiple times with XDR and Transactions). So would be nice to use some basic duck-typing check for ScVal.

[leighmcculloch]

Because a lot of the types in the SDK offer static functions, to aid with discoverability we could keep the from function, but make it so there is only one, and that the from function mirrors the constructor, so there's really only one function and one concept, it's just available either as a constructor or as a static function.

[leighmcculloch]

new Int(12, 'i128'); 
new Int('12', 'u64'); 
new Int(12n, 'u128'); 
new Int(scv); 

Would the second argument, the type, be required for non-ScVal inputs?

[orbitlens]

Not necessarily. MongoDB driver, for example, automatically determines an appropriate type (int32, int64, etc) from the number. So, if the number can't fit into i32, the automatically assigned type would be i64, and so on. But yeah, I agree that in the context of smart contracts, the type for non-ScVal value should be provided explicitly.

[Shaptic]

@orbitlens that's the way that ScInt was designed initially, but XdrLargeInt required the type so it came first (bad call at the time in retrospect), so making XdrLargeInt -> Int and also switch the parameter order (values, type?) instead of (type, values) would be a bigger breaking change to any XdrLargeInt users that I wanted to avoid, hence the fromValue. Ideally yes, fromValue could be absorbed into the constructor, but it'd be annoying to migrate, and letting it be (undefined, [values]) is ugly :/

If people don't mind migrating we can do it this way, then also allow values to be an ScVal and drop both froms. That'd be the ideal case. Worth it?

[kalepail]

I've reviewed and don't have anything substantive to add. I haven't used the existing method much and the proposed change seems clean 👍