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

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Jun 30, 2023

This enhanced the nativeToScVal helper, allowing more flexibility in the output types.

Specifically, this lets you:

  • build scvU32 and scvI32 types
  • build scvSymbols from strings and bytes
  • build scvStrings from bytes
  • build scvVecs with specific types (i.e. recursive type specification)
  • build scvMaps with specific types for both keys and values (via a simple type specification format (e.g. type: { field: [ keyType?, valueType? ] } for each entry in the map)

Related: #637

src/numbers/index.js Outdated Show resolved Hide resolved
src/scval.js Show resolved Hide resolved
@Shaptic Shaptic mentioned this pull request Jul 3, 2023
* @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.

@Shaptic Shaptic merged commit 792b5d2 into soroban Jul 3, 2023
@Shaptic Shaptic deleted the moar-scval-fixes branch July 3, 2023 22:15
Shaptic added a commit that referenced this pull request Sep 12, 2023
* Correctly set `minAccountSequence` in `TransactionBuilder` for large values (#539)
* Add support of CAP40 ed25519SignedPayload signer for SetOptionOp (#542)
* Add TypeScript interfaces for Ed25519SignedPayload signers
* Coalesce all xLM, Xlm, etc. => 'XLM' native asset code (#546)
* make sure sodium is not an empty object in service workers (#567)
* Add futurenet passphrase for soroban. (#550)
* Include standalone network passphrase (#555)
* Soroban auth next updates (#570)
* Add strkey parsing for contracts, and Address helper for building ScAddresses (#572)
* Update xdr to include HashIdPreimageContractAuth.nonce (#577)
* Add Address.toBuffer method (#578)
* Soroban value overhaul (#582)
* Update generated XDR to latest version. (#587)
* Update XDR and contract invocations to conform to the latest version (Preview 9). (#601)
* added soroban transaction data as tx builder param (#604)
* Add Contract support for strkey-style contract IDs (#612)
* Add Contract.address() method (#614)
* Add wrappers to easily deal with the myriad of `ScVal` integer types. (#620)
* Upgrade `js-xdr` to v3.0.0 to support `bigint` encoding.
* Add wrappers to easily convert between `ScVal` and native types. (#630)
* Update XDR to support Preview 10 (#633)
* Fix TypeScript definition for `invokeHostFunction` options (#635)
* Add fully-qualified `Operation` type to XDR type definitions. (#591)
* Adds Operation.isValidAmount jsdoc (#609)
* Removes the dependency on `crc` by calculating checksum in strkey (#621)
* Improve type flexibility when building `ScVal`s from native types (#638)
* Fix Preview 10 contract invoke & release `v10.0.0-soroban.3` (#642)
* Add GitHub Action that compares before-and-after bundle sizes. (#644)
* Make opinionated judgements about string/symbol decoding (#645)
* Make Node 16 the minimum version. (#651)
* Add standalone/futurenet passphrases to jsdoc (#654)
* Clean up unnecessary dependency entries (#652)
* Add `SorobanDataBuilder` builder pattern to prepare transactions easier (#660)
* Adds a way to create a `TransactionBuilder` from an existing transaction. (#656)
* Add basic contract event parsing into a human-readable structure (#659)
* Drop support for deprecated hex contract IDs (#658)
* Add helpers method to build authorization entries. (#663)
* Fix TypeScript definition for new static `TransactionBuilder` constructor (#665)
* Add ability to clear out operations from the transaction builder (#670)
* Add an existing method to XdrLargeInt, jsdoc/types fixup (#668)
* Expand contract footprint with contract code ledger key. (#662)
* Decode base64-encoded strings in new SorobanDataBuilder (#673)
* Add ability to modify and retrieve the resource footprint (#680)
* Fixes error messages for required amount parameters (#679)
* Add `Asset.contractId()` helper to predict contract IDs (#684)
* Upgrade dependencies to their latest minor/patch version (#685)
* Improves `SorobanDataBuilder` construction and adds a getter (#686)
* Add JSON-ification of Soroban invocation trees. (#669)
* Add utilities for formatting token amounts (`formatTokenAmount` & `parseTokenAmount`) (#667)
* Merges the final Protocol 20 XDR (i.e. for testnet) into `soroban` (#688)
* Add support to convert strings to large integer and address `ScVal`s (#690)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Paul Bellamy <paul@stellar.org>
Co-authored-by: George <Shaptic@users.noreply.github.com>
Co-authored-by: Jun Luo <4catcode@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: OrbitLens <33724849+orbitlens@users.noreply.github.com>
Co-authored-by: Piyal Basu <pbasu235@gmail.com>
Co-authored-by: George Kudrayvtsev <george@stellar.org>
Co-authored-by: shawn <sreuland@users.noreply.github.com>
Co-authored-by: Silence <35656692+silence48@users.noreply.github.com>
Co-authored-by: Chad Ostrowski <221614+chadoh@users.noreply.github.com>
Co-authored-by: jeesun 지선 <jeesunikim@users.noreply.github.com>
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.

2 participants