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

Improve type flexibility when building ScVals from native types #638

Merged
merged 11 commits into from
Jul 3, 2023
17 changes: 7 additions & 10 deletions src/numbers/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { XdrLargeInt } from './xdr_large_int';

import { Uint128 } from './uint128';
import { Uint256 } from './uint256';
import { Int128 } from './int128';
import { Int256 } from './int256';

export { Uint256, Int256, Uint128, Int128 };

export { Uint128 } from './uint128';
export { Uint256 } from './uint256';
export { Int128 } from './int128';
export { Int256 } from './int256';
export { ScInt } from './sc_int';
export { XdrLargeInt };

Expand All @@ -21,8 +18,8 @@ export { XdrLargeInt };
* let scv = contract.call("add", x, y); // assume it returns an xdr.ScVal
* let bigi = scValToBigInt(scv);
*
* new ScInt(bigi); // if you don't care about types, and
* new XdrLargeInt('i128', bigi); // if you do
* new ScInt(bigi); // if you don't care about types, and
* new XdrLargeInt('i128', bigi); // if you do
* ```
*
* @param {xdr.ScVal} scv - the raw XDR value to parse into an integer
Expand All @@ -31,7 +28,7 @@ export { XdrLargeInt };
* @throws {TypeError} if the `scv` input value doesn't represent an integer
*/
export function scValToBigInt(scv) {
const type = scv.switch().name.slice(3).toLowerCase();
const type = scv.switch().name.slice(3).toLowerCase(); // if it ain't broke...
sreuland marked this conversation as resolved.
Show resolved Hide resolved

switch (scv.switch().name) {
case 'scvU32':
Expand Down
155 changes: 131 additions & 24 deletions src/scval.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* native JavaScript types.
*
* @example
* ```js
* import { nativeToScVal, scValToNative, ScInt, xdr } from 'stellar-base';
*
* let gigaMap = {
Expand Down Expand Up @@ -32,11 +33,13 @@
*
* // Similarly, the inverse should work:
* scValToNative(scv) == gigaMap; // true
* ```
*/

import xdr from './xdr';

import { Address } from './address';
import { Contract } from './contract';
import { ScInt, scValToBigInt } from './numbers/index';

/**
Expand All @@ -54,7 +57,8 @@ import { ScInt, scValToBigInt } from './numbers/index';
* - number/bigint -> the smallest possible XDR integer type that will fit the
* input value (if you want a specific type, use {@link ScInt})
*
* - {@link Address} -> scvAddress (for contracts and public keys)
* - {@link Address} or {@link Contract} -> scvAddress (for contracts and
* public keys)
*
* - Array<T> -> scvVec after attempting to convert each item of type `T` to an
* xdr.ScVal (recursively). note that all values must be the same type!
Expand All @@ -67,12 +71,33 @@ import { ScInt, scValToBigInt } from './numbers/index';
* type which will force a particular interpretation of that value.
*
* Note that not all type specifications are compatible with all `ScVal`s, e.g.
* `toScVal("string", {type: i256})` will throw.
* `toScVal("a string", {type: "i256"})` will throw.
*
* @param {any} val a native (or convertible) input value to wrap
* @param {object} [opts] an optional set of options to pass which allows you
* to specify a type when the native `val` is an integer-like (i.e.
* number|bigint) type
* @param {any} val - a native (or convertible) input value to wrap
* @param {object} [opts] - an optional set of hints around the type of
* conversion you'd like to see
* @param {string} [opts.type] - there is different behavior for different input
Copy link
Contributor

Choose a reason for hiding this comment

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

this may get to be an uber-method to maintain all type permutations for native types in one method. As of now, should docs explicitly show a matrix of valid native type to opt.types? or callers need to look at code.

just for consideration later, ways to cordon options directly to their native types, like a TypeConverter class with toScVal/fromScVal or functional like passing converter function instead like nativeToScVal(val, converterFn). I think recall seeing in past reviews that baking this type of conversions into generated ScVal from xdr was not reasonable.

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 agree that there's a risk here of permutation blow-up, BUT I'm hoping that this can stay relatively lightweight. The matrix is sparse, so to speak, in that many types have one or two possibilities at most. So hopefully it doesn't get ugly. (In fact, you could even argue that some of the permutations are the fault of design mistakes on the XDR level, like ScVal.scvStrings taking a Buffer, but I digress...)

With the discussion on custom, user-defined types in #637, you very well may be right that this can end up exploding into an uber method... so let's cross that bridge if we get to it? I don't want to over-invest a lot of architectural thought into this until we get community feedback on whether or not this is even a useful abstraction to provide in the first place. I think of this as a "minimum high-level implementation" to try to help downstream folks not deal with XDR so much, more-so than a perfect v0 solution.

* types for `val`:
*
* - when `val` is an integer-like type (i.e. number|bigint), this will be
* forwarded to {@link ScInt} or forced to be u32/i32.
*
* - when `val` is an array type, this is forwarded to the recursion
*
* - when `val` is an object type (key-value entries), this should be an
* object in which each key has a pair of types (to represent forced types
* for the key and the value), where `null` (or a missing entry) indicates
* the default interpretation(s) (refer to the examples, below)
*
* - when `val` is a string type, this can be 'string' or 'symbol' to force
* a particular interpretation of `val`.
*
* - when `val` is a bytes-like type, this can be 'string', 'symbol', or
* 'bytes' to force a particular interpretation
*
* As a simple example, `nativeToScVal("hello", {type: 'symbol'})` will
* return an `scvSymbol`, whereas without the type it would have been an
* `scvString`.
*
* @returns {xdr.ScVal} a wrapped, smart, XDR version of the input value
*
Expand All @@ -82,9 +107,35 @@ import { ScInt, scValToBigInt } from './numbers/index';
* types, custom classes)
* - the type of the input object (or some inner value of said object) cannot
* be determined (via `typeof`)
* - the type you specified (via `opts.type`) is incompatible with the value
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
* you passed in (`val`), e.g. `nativeToScVal("a string", { type: 'i128' })`.
*
* TODO: Allow users to force types that are not direct but can be translated,
* i.e. forcing a `Uint8Array` to be encoded as an ScSymbol or ScString.
* @example
*
* ```js
* nativeToScVal(1000); // gives ScValType === scvU64
* nativeToScVal(1000n); // gives ScValType === scvU64
* nativeToScVal(1n << 100n); // gives ScValType === scvU128
* nativeToScVal(1000, { type: 'u32' }); // gives ScValType === scvU32
* nativeToScVal(1000, { type: 'i125' }); // gives ScValType === scvI256
* nativeToScVal("a string"); // gives ScValType === scvString
* nativeToScVal("a string", { type: 'symbol' }); // gives scvSymbol
* nativeToScVal(new Uint8Array(5)); // scvBytes
* nativeToScVal(new Uint8Array(5), { type: 'symbol' }); // scvSymbol
* nativeToScVal(null); // scvVoid
* nativeToScVal(true); // scvBool
* nativeToScVal([1, 2, 3]); // gives scvVec with each element as scvU64
* nativeToScVal([1, 2, 3], { type: 'i128' }); // scvVec<scvI128>
* nativeToScVal({ 'hello': 1, 'world': [ true, false ] }, {
* type: {
* 'hello': [ 'symbol', 'i128' ],
* }
* })
* // gives scvMap with entries: [
* // [ scvSymbol, scvI128 ],
* // [ scvString, scvArray<scvBool> ]
* // ]
* ```
*/
export function nativeToScVal(val, opts = {}) {
switch (typeof val) {
Expand All @@ -94,22 +145,42 @@ export function nativeToScVal(val, opts = {}) {
}

if (val instanceof xdr.ScVal) {
return val;
return val; // should we copy?
}

if (val instanceof Address) {
return val.toScVal();
}

if (val instanceof Uint8Array) {
return xdr.ScVal.scvBytes(Uint8Array.from(val));
if (val instanceof Contract) {
return val.address().toScVal();
}

if (val instanceof Uint8Array || Buffer.isBuffer(val)) {
const copy = Uint8Array.from(val);
switch (opts?.type ?? 'bytes') {
case 'bytes':
return xdr.ScVal.scvBytes(copy);
case 'symbol':
return xdr.ScVal.scvSymbol(copy);
case 'string':
return xdr.ScVal.scvString(copy);
default:
throw new TypeError(
`invalid type (${opts.type}) specified for bytes-like value`
);
}
}

if (Array.isArray(val)) {
if (val.length > 0 && val.some((v) => typeof v !== typeof v[0])) {
throw new TypeError(`array value (${val}) must have a single type`);
if (val.length > 0 && val.some((v) => typeof v !== typeof val[0])) {
throw new TypeError(
`array values (${val}) must have the same type (types: ${val
.map((v) => typeof v)
.join(',')})`
);
}
return xdr.ScVal.scvVec(val.map(nativeToScVal));
return xdr.ScVal.scvVec(val.map((v) => nativeToScVal(v, opts)));
}

if ((val.constructor?.name ?? '') !== 'Object') {
Expand All @@ -121,18 +192,49 @@ export function nativeToScVal(val, opts = {}) {
}

return xdr.ScVal.scvMap(
Object.entries(val).map(
([k, v]) =>
new xdr.ScMapEntry({ key: nativeToScVal(k), val: nativeToScVal(v) })
)
Object.entries(val).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)
});
})
);

case 'number':
case 'bigint':
return new ScInt(val, opts).toScVal();
switch (opts?.type) {
case 'u32':
return xdr.ScVal.scvU32(val);

case 'i32':
return xdr.ScVal.scvI32(val);

default:
break;
}

return new ScInt(val, { type: opts?.type }).toScVal();

case 'string':
return xdr.ScVal.scvString(val.toString());
switch (opts?.type ?? 'string') {
case 'string':
return xdr.ScVal.scvString(val);

case 'symbol':
return xdr.ScVal.scvSymbol(val);

default:
throw new TypeError(
`invalid type (${opts.type}) specified for string value`
);
}

case 'boolean':
return xdr.ScVal.scvBool(val);
Expand Down Expand Up @@ -160,7 +262,7 @@ export function nativeToScVal(val, opts = {}) {
* - map -> key-value object of any of the above (via recursion)
* - bool -> boolean
* - bytes -> Uint8Array
* - string, symbol -> string
* - string, symbol -> string|Uint8Array
*
* If no conversion can be made, this just "unwraps" the smart value to return
* its underlying XDR value.
Expand Down Expand Up @@ -213,9 +315,14 @@ export function scValToNative(scv) {
return scv.value();

case xdr.ScValType.scvString().value:
case xdr.ScValType.scvSymbol().value:
// FIXME: Is this the right way to handle it being string|Buffer?
return String(scv.value());
case xdr.ScValType.scvSymbol().value: {
const v = scv.value(); // string|Buffer
if (Buffer.isBuffer(v)) {
// trying to avoid bubbling up problematic Buffer type
return Uint8Array.from(v);
}
return v; // string
}

// these can be converted to bigint
case xdr.ScValType.scvTimepoint().value:
Expand Down
64 changes: 62 additions & 2 deletions test/unit/scval_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ describe('parsing and building ScVals', function () {
],
[xdr.ScVal.scvString('hello there!'), 'hello there!'],
[xdr.ScVal.scvSymbol('hello'), 'hello'],
[xdr.ScVal.scvSymbol(Buffer.from('hello')), 'hello'],
[xdr.ScVal.scvString(Buffer.alloc(32, '\xba')), '\xba'.repeat(16)],
[xdr.ScVal.scvString(Buffer.from('hello')), Buffer.from('hello')], // ensure no conversion
[xdr.ScVal.scvSymbol(Buffer.from('hello')), Buffer.from('hello')], // ensure no conversion
[
new StellarBase.Address(kp.publicKey()).toScVal(),
(actual) => actual.toString() === kp.publicKey()
Expand Down Expand Up @@ -146,4 +146,64 @@ describe('parsing and building ScVals', function () {
}
});
});

it('converts native types with customized types', function () {
[
[1, 'u32', 'scvU32'],
[1, 'i32', 'scvI32'],
[1, 'i64', 'scvI64'],
[1, 'i128', 'scvI128'],
[1, 'u256', 'scvU256'],
['a', 'symbol', 'scvSymbol'],
['a', undefined, 'scvString'],
[Buffer.from('abcdefg'), undefined, 'scvBytes'],
[Buffer.from('abcdefg'), 'string', 'scvString'],
[Buffer.from('abcdefg'), 'symbol', 'scvSymbol']
].forEach(([input, typeSpec, outType]) => {
let scv = nativeToScVal(input, { type: typeSpec });
expect(scv.switch().name).to.equal(
outType,
`in: ${input}, ${typeSpec}\nout: ${JSON.stringify(scv, null, 2)}`
);
});

let scv;

scv = nativeToScVal(['a', 'b', 'c'], { type: 'symbol' });
expect(scv.switch().name).to.equal('scvVec');
scv.value().forEach((v) => {
expect(v.switch().name).to.equal('scvSymbol');
});

scv = nativeToScVal(
{
hello: 'world',
goodbye: [1, 2, 3]
},
{
type: {
hello: ['symbol', null],
goodbye: [null, 'i32']
}
}
);
let e;
expect(scv.switch().name).to.equal('scvMap');

e = scv.value()[0];
expect(e.key().switch().name).to.equal('scvSymbol', `${JSON.stringify(e)}`);
expect(e.val().switch().name).to.equal('scvString', `${JSON.stringify(e)}`);

e = scv.value()[1];
expect(e.key().switch().name).to.equal('scvString', `${JSON.stringify(e)}`);
expect(e.val().switch().name).to.equal('scvVec', `${JSON.stringify(e)}`);
expect(e.val().value()[0].switch().name).to.equal(
'scvI32',
`${JSON.stringify(e)}`
);
});

it('throws on arrays with mixed types', function () {
expect(() => nativeToScVal([1, 'a', false])).to.throw(/same type/i);
});
});