Simplify how to convert from ScVal types into BigInts/Numbers

[leighmcculloch]

Is your feature request related to a problem? Please describe.

The APIs for working with XDR ScVal number types are reasonably confusing and non-obvious. There are also multiple ways to do the same thing.

For example there are three ways to get at / work with number values:

1. Use the scValToBigInt function:

   // ScVal -> BigInt
   const bigint = scValToBigInt(scval);

2. Use the ScInt type:

   // ScVal -> BigInt
   const bigint = ScInt.fromScVal(scval);

   // BigInt -> ScVal
   const scval = new ScInt(bigint).toI128();

   Another problem with ScInt is that it looks like an XDR type, but it isn't.

3. Use the XdrLargeInt type. This type should probably not be exported as it seems like an implementation detail?

   // ScVal -> BigInt
   const bigint = new XdrLargeInt(scIntType, [scval.value().lo(), scval.value().hi()]).toBigInt();

   Another problem with XdrLargeInt is that there's another type LargeInt which serves another purpose.

Describe the solution you'd like

Functions on ScVal. Close proximity to ScVal so that the functionality is discoverable, and less new concepts (XdrLargeInt, ScInt) are introduced).

// BigInt -> ScVal
ScVal.newI128(bigint)
ScVal.newU128(bigint)
ScVal.newI64(bigint)
...

// ScVal -> BigInt
const bigint = scval.toBigInt();
const number = scval.toNumber(); // with check

As a bonus, ScVal already contains functions for more advanced usage creating the values from parts:

ScVal.scvI128(
  new xdr.Int128Parts({
    hi: new xdr.Int64(hi64),
    lo: new xdr.Uint64(lo64)
  })
)

Or the simpler variants can be added:

// Parts -> ScVal
ScVal.newI128FromParts(hi, lo)

// ScVal -> Parts
const parts = scval.toI128Parts()

Describe alternatives you've considered

Another option would be to focus on using just one of these methods, such as combining ScInt and XdrLargeInt so all conversions (simple and advanced) are in one type, and removing scValToBigInt since it's superfluous.

Additional context

Thinking about this after seeing the following question in Discord:

• https://discord.com/channels/897514728459468821/1189109010754977802

Historically we've tried to pull people away from the XDR, and hide the XDR where possible behind nicer APIs. With Soroban folks need to embrace the XDR more than they did with classic operations. Also, in this case the APIs we've pulled people from to are still relatively confusing, and we can introduce less concepts by pushing people back to the XDR type and adding functions to simplify interaction on the XDR type.

(I am more interested in solving the problem than with the solution I suggest above, and definitely defer to others more qualified for better solutions.)

[Shaptic]

Thanks for drafting this, @leighmcculloch. I do agree that the APIs are suboptimal, but they were a huge improvement over building the XDR values from scratch at the time.

First, some rebuttals, and then alignment towards a solution:

const bigint = ScInt.fromScVal(scval);

Thanks for bringing this up, because it actually doesn't exist 😅 it's actually a documentation error (fixed in #723). So, on the brighter side, there are less ways.

Use the XdrLargeInt type. This type should probably not be exported as it seems like an implementation detail?

This is the lowest-level way to create the value and was specifically requested in the original implementation. Their difference is noted in the documentation:

new ScInt(bigi);               // if you don't care about types, and
new XdrLargeInt('i128', bigi); // if you do

I'm not sure I follow the arguments about XDR types: namespacing makes this clear. Nothing outside of the xdr module is, well, raw XDR.

As a bonus, ScVal already contains functions for more advanced usage creating the values from parts:

The whole point of these abstractions was to NOT do this, because it's super duper error-prone and gross. 99.9% of developers will do it wrong (we literally did it wrong ourselves several times in the VM). And if you really want to, this is what XdrLargeInt is for.

So, in summary, there really is only a single way to go from ScVal to bigint, which is scValToBigInt. If you already have an ScInt, it's different, of course. The inverse direction (bigint|number|string to ScVal) has more than one way, but imo this isn't really problematic: nativeToScVal (generic) and ScInt (specific) are on different abstraction layers.

---

Now, for the proposed APIs:

Functions on ScVal. Close proximity to ScVal so that the functionality is discoverable, and less new concepts (XdrLargeInt, ScInt) are introduced).

I think this is a great idea: it was mentioned in the original PR, but you can see in that thread that it's not exactly trivial to extend generated XDR, unfortunately.

Historically we've tried to pull people away from the XDR, and hide the XDR where possible behind nicer APIs. With Soroban folks need to embrace the XDR more than they did with classic operations. Also, in this case the APIs we've pulled people from to are still relatively confusing, and we can introduce less concepts by pushing people back to the XDR type and adding functions to simplify interaction on the XDR type.

I strongly disagree with embracing the XDR more if we can avoid it. The Soroban XDR structures are gross and error-prone and this is exacerbated by js-xdr being completely unintuitive to interact with (aka _attributes and all of that nonsense). I think many devs would agree that scValToNative and nativeToScVal are so much nicer than dealing with the generated code.

So I agree that attaching it to the structures would be the neatest approach, and I want to look into this more, but it's not a trivial fix unfortunately :/