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 :sweat_smile: it's actually a documentation error (fixed in https://github.com/stellar/js-stellar-base/pull/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 :/

[VyuduInc]

Hey Leigh,

This is one of those classic SDK pain points ... too many ways to do the same thing, and none of them feel quite right. ScVal number conversions should be simple, but right now it’s like wandering through a type forest with three different compasses.

I’m down to help cut through the clutter. ScVal instance methods like .toBigInt() and .toNumber()? That’s the move. Keeping the API surface tight, discoverable, and intuitive ,, no need to juggle ScInt, XdrLargeInt, or whatever secret boss fight is hiding in LargeInt.

The fact that ScVal already supports .scvI128() with part-wise construction is a perfect foundation. Wrapping that in ScVal.newI128(bigint) or .newI128FromParts(hi, lo) gives us both the low-level power and high-level simplicity ... best of both blocks.

I’ve been building in the blockchain world since 2017 ,, Forbes Tech Council writer, investor, and dev.

Here’s my work if you wanna check the energy: https://vyudu-links.netlify.app/

Cheers,

[leighmcculloch]

js-xdr being completely unintuitive to interact

🤔 Maybe it's time we invest in a new JS XDR.

scValToNative and nativeToScVal are so much nicer

🤔 In some ways yes, but in others they're surprising, another language to learn to speak. The surprising part is that some types convert cleanly, while others require specifying. E.g. Symbol vs String. Unsigned vs signed integer.

no need to juggle ScInt, XdrLargeInt

👍🏻 While these types may be abstractions, they are not intuitive, and knowledge in either standard JS types or XDR doesn't help you understand what they do because they're a new thing with a third interface.

ScVal instance methods like .toBigInt() and .toNumber()

👍🏻 That seems intuitive to me, and explicit, less surprises.

ScVal.newI128(bigint)

👍🏻 Although, I think we can modify the existing function so as to reduce the surface area, reduce things that folks need to discover, whilst offering intuitive native types:

-xdr.ScVal.scvI128(value: xdr.Int128Parts): xdr.ScVal
+xdr.ScVal.scvI128(value: xdr.Int128Parts | BigInt): xdr.ScVal

This last example requires modifying how the JS code is generated from XDR, but I think that's doable. The Rust code generated from the XDR modifies some code like this too.

I've updated the proposal in the description to use this.

[Shaptic]

invest in a new JS XDR

I would 👍 this 1000 times if I could - the current approach may have made sense 10 years ago when it was written (literally), but the JS/TS ecosystem has come a long way since then. The hard part is (a) backwards compatibility (if every JS dev had to rewrite their XDR manipulation code from scratch they'd have a conniption), (b) coming up with a sane, type-safe design for the thing (I've tried and failed on paper several times), and (c) allocating the bandwidth to actually do it.

another language to learn to speak

Very valid, but it's at least an easier language to speak than XDR, I think 😛 it pretty much comes down to "intuitive when possible, manual (but easier) when necessary." I can't really think of an intuitive way to present signed vs. unsigned and different bit sizes to a consumer that doesn't require some level of manual specificity.

knowledge in either standard JS types or XDR doesn't help

Again, very valid, but that's a product of neither JS nor XDR having these concepts in the first place. Just the other day I needed to negate a large integer and it wasn't obvious whatsoever. bigint|string is basically the great unifier, but even then at some point you do need to match your function signature to your native value...

+xdr.ScVal.scvI128(value: xdr.Int128Parts | BigInt): xdr.ScVal

I can totally get on board with this - I think it's the best path forward. I'd go a step further and allow string (and maybe even number), but I do agree that the best approach is embedding it into the object itself.

However, I think we need to be aware of our own bias towards being comfortable with the XDR vs. a fresh set of eyes who feels weird any time they use xdr.*. I think dropping scValToBigInt (this really only exists because it was written before scValToNative) and ScInt in favor of explicit use of XdrLargeInt (which yes, could use a rename) would alleviate a lot of the confusion in general about the multiple API surfaces. Wdyt?

[leighmcculloch]

I think dropping scValToBigInt (this really only exists because it was written before scValToNative) and ScInt in favor of explicit use of XdrLargeInt (which yes, could use a rename) would alleviate a lot of the confusion in general about the multiple API surfaces. Wdyt?

Dropping the others, and renaming XdrLargeInt to simply Int, and adding support for u32/i32 types so that it covers all use of numbers, would be a net benefit because it'd result in less options.

But I think the way to insulate developers from interacting with contract XDR types like ScVal is to code generate good clients that accept regular JS types for all the inputs. In those cases, yeah, no XDR should be required because the code generates knows what XDR types to convert to.

For the use cases where developers actually need to build an ScVal, they're in the XDR, and so calling ScVal.scvI128(...) is more obvious than using other helper types.

I'd go a step further and allow string (and maybe even number)

👍🏻