Replace ScObject with ScOption. Unify everything under ScVal.

[paulbellamy]

While investigating stellar/stellar-cli#330, it turns out there's currently no way to explicitly represent Option<Symbol> (or Option<any other primitive>) in the xdr. Instead any ScVal might be an ScStatic::Void, so everything is an optional.

The confusion I had was because ScObject is doing double-duty gets misused as an ‘Optional’ type. ScObject is also a weird performance-abstraction-leak thing.

I’m proposing:

1. Get rid of ScObject and just have everything under ScVal.
   • Internal performance optimizations in core are not something clients should care about.
   • The division makes working with ScVals more complicated to explain and implement correctly in clients.
2. Add ScOption to represent Option.
3. Update ScSpecType stuff to match? It looks like this is already how ScSpecType works...

[graydon]

I would not say "never" to this, but I'm generally fairly opposed.

Concerning "getting rid of ScObject", there are two separate reasons to keep it:

1. The small value / big object split is inherent to Soroban's conceptual model. Values cross the guest/host barrier, objects do not. I don't think hiding this distinction is going to help people understand what's going on (I'm already worried the SDK is doing too much work hiding the reality by making immutable host objects appear as mutable guest objects).
2. It's not just a perf optimization in core. ScVal is a recursive type (ScVal contains ScObject which contains ScVec which contains ScVal...) and currently due to the way xdrpp implements recursive types there has to be an XDR Option<> in there somewhere -- it's the only XDR type represented as a pointer. Otherwise the type is infinite-sized and won't compile. This could theoretically change someday -- there's a pending xdrpp branch that changes it and happens to include having-to-upgrade-core-to-C++20 -- but at present we literally cannot compile a recursive type without such an edge in it. If you land this change today, core will not compile.

Concerning changing Option from a compromised type (represented using maybe-Void RawVal) to some kind of first-class type constructor (i.e. one that can round-trip a type like Option<Option<T>> through ScVal) -- the current fate of Option<> is an intentional compromise in the representation repertoire to fit into the limited bit-packing space available in RawVal, one of two similar intentional compromises: we also can't/don't represent Result<Result<T,U>,V> because we collapse all Result<>s of any nesting into a single-level Status, losing the nesting structure. The alternative to this would not be to make new ScVal variants but new ScObject variants, because as a recursive type they must indirect through something non-fixed-size, and our model has ScVal mapping to a fixed-size 64-bit RawVal (i.e. even if we "reserved a bit" to indicate optional-ness, that'd only let you round-trip Option<u32> not Option<Option<u32>> or Option<Option<Option<u32>>> -- you'd need to reserve unbounded bits, which is to say, it would actually need to be an object that can point to other objects). We choose not to do this because shallow versions of these types are fairly ubiquitous and it's performance-penalizing to orient the design around the much rarer nested case. If a user truly needs a nested structure they can round-trip, they have vectors and maps with which to build their own.

[leighmcculloch]

I think this proposal contains a couple of things we can discuss and decide on independently:

1. Flattening the Val/Object XDR structure.

   I'm moderately in favor of this. I say moderately because while I think it is much better for the user, the cost to rework the type system inside env might be substantial.

   I don't think hiding this distinction is going to help people understand what's going on

   I think the main issue isn't to help people understand what's going on, for a lot of developers I don't think they need to know what is going on here, and I think that's @paulbellamy's point. For a developer interacting with the network they shouldn't need to see some types over here akin to primitives and some types over there akin to objects. Ideally for a developer operating in user land there are just the built-in types, and that's it. While RawVal and Object must exist in the guest and host system, that doesn't mean that structure has to leak into the structure of the XDR.

   I know the separation of primitive and object types is not new, it reminds me of Java's type system a lot since every Java developer in early versions of Java had to know that primitive types were different to object types. But it seems distracting for Soroban developers to have to learn about this.

   Unfortunately it's unlikely we'll abstract it away in user-land SDKs that have to build XDR. And it's difficult to hide it in things like JSON that is generated from XDR. It keeps coming up as a question from folks, why are some types nested under "Object" and why aren't the other types.

   I don't think env benefits from a performance benefit of this either, but rather it's mostly code organization. Type conversations in env are organized with object types being convertible by the host and val types being convertible by themselves. Putting them into one XDR type will require rewriting a lot of XDR type conversion code, and it would prevent us from being able to have infallible conversions from types like Object to ScObject and vice-versa.

2. Introducing a first-class Option<> type.

   In the past I advocated for us having a first-class Option type as well, but I've pleasantly surprised with how well the compromised type system has worked out, for Option, but especially for Result and our error handling.

   After pondering this again, and pondering the success we've had with the simpler system we have going, it's really unclear to me if the change is worth it given the costs, or if there is a substantial benefit from making this change. What we have works. I think what we have is a good balance between a perfect representation and an efficient/useful one.

   it turns out there's currently no way to explicitly represent Option (or Option) in the xdr

   I'm confused why you can't represent Option in a contract. Here's a contract that returns an Option<Symbol>:

   #![no_std]
   use soroban_sdk::{contractimpl, symbol, Symbol};
   
   pub struct Contract;
   
   #[contractimpl]
   impl Contract {
       pub fn some() -> Option<Symbol> {
           Some(symbol!("gday"))
       }
       pub fn none() -> Option<Symbol> {
           None
       }
   }
   
   #[cfg(test)]
   mod test {
       extern crate std;
   
       use soroban_sdk::Env;
   
       use crate::{Contract, ContractClient};
   
       #[test]
       fn test_some() {
           let e = Env::default();
           let contract_id = e.register_contract(None, Contract);
           let client = ContractClient::new(&e, &contract_id);
   
           let none = client.none();
           assert_eq!(none, None);
       }
   
       #[test]
       fn test_none() {
           let e = Env::default();
           let contract_id = e.register_contract(None, Contract);
           let client = ContractClient::new(&e, &contract_id);
   
           let none = client.none();
           assert_eq!(none, None);
       }
   }

   And here's some tests that demonstrate Option<> in XDR working:

    #[cfg(test)]
    mod test_xdr {
        extern crate std;
   
        use soroban_sdk::{
            symbol,
            xdr::{ScStatic, ScVal},
            Env, IntoVal, RawVal, Symbol, TryIntoVal,
        };
   
        #[test]
        fn test_some() {
            let e = Env::default();
            let some: Option<Symbol> = Some(symbol!("gday"));
            let val: RawVal = some.into_val(&e);
            let val: ScVal = val.try_into_val(&e).unwrap();
            assert_eq!(val, ScVal::Symbol("gday".try_into().unwrap()));
        }
   
        #[test]
        fn test_none() {
            let e = Env::default();
            let none: Option<Symbol> = None;
            let val: RawVal = none.into_val(&e);
            let val: ScVal = val.try_into_val(&e).unwrap();
            assert_eq!(val, ScVal::Static(ScStatic::Void));
        }
    }

   Note we did previously discuss adding another static value for Option's None, but we decided against it given that void existed and was usable. See:

   • Add STATIC_NONE static type for use with Optional<T> rs-soroban-env#105

[graydon]

Flattening the Val/Object XDR structure

Except if you do that, now instead of only one Option edge in the type graph, you have an Option on any edge to a type that contains a nested Val. So like Val<Option> and Val<Option>. Because of the thing where xdrpp needs an option interposed in a recursive type cycle.

[leighmcculloch]

Except if you do that, now instead of only one Option edge in the type graph, you have an Option on any edge to a type that contains a nested Val.

I think that's okay. Most users don't experience any problems because of that, at least not problems that I think are worse than the nesting creates. If we want to address that problem we should be addressing it in xdrpp. I don't think doing this needs to be contingent on addressing that problem though.

[graydon]

Ok. I'm .. still fairly opposed even to this step. I think it's a mistake to hide something that's fundamental to the way the system works from users. You say users don't need to know, but I think that's an overstatement. End-end users operating contracts from wallets and GUIs won't see either way. Users who are writing contracts do need to know, it's part of how the system works and will be relevant when debugging. The only users who might not need to know are dapp devs who are only working in JS and not writing contract code. I don't think it's right to spend what will likely be several weeks of work moving things around and rewriting in order to optimize for them at the expense of all contract devs, and the maintainers of soroban itself (the distinction absolutely runs through the implementation).

[leighmcculloch]

Users who are writing contracts do need to know, it's part of how the system works and will be relevant when debugging.

The SDK tries pretty hard, and does a pretty decent job of making it unnecessary to know this. For sure if you're debugging a Vec or a Map, all you'll see is an integer and probably be confused. But if you println! a vec or map out in a test, or even assert_eq! on two values that fail to equal, the string outputs fully render all of the internal elements. So I think we've gone a long way down the path of obfuscating that Object exists to contract developers.

I think it will be likely that there will be a subset of contract developers who understand Object exists – the advanced / experts – and there will be the larger set of folks who just haven't ever needed to know, and may not ever need to know.

I think there's a benefit here, but it's a benefit with tradeoffs.

Another possibility is we do build a usability layer into SDKs so that people aren't fiddling with these details, but that is also more code to build and maintain.

[paulbellamy]

Nice breakdown between the two discussion, @leighmcculloch. Thanks.

After reflection, I think 1 (flattened obj structure) would be a nice-to-have, and I'm not sure about 2 (option) anymore. Related feedback from the hackathon (stellar/stellar-docs#485), so I'm clearly not the only one confused by the mismatch between the xdr and the rust type systems.

[graydon]

Spacing out numbers like this isn't ideal. We reuse these numbers as object code tags which means they get encoded into the instruction stream as literals. Bigger numbers => larger codesize.

[graydon]

SCVec and SCMap being recursive types here have to be SCVec* amd SCMap*

[graydon]

Another difficulty (or possibly: opportunity) with this is that The Current Plan includes adding subtypes to objects: stellar/rs-soroban-env#583

If we were to not have "object" represented in the XDR anymore, we'd either add subtypes to values, or add them case-by-case to the value cases that are object-like / want-to-have-subtypes. Or literally duplicate those cases for each subtype we have in mind at the representation level, eg have a separate ScVal::Text and ScVal::Bytes that only differ in interpretation, both have the same representation / be able to be passed to host functions expecting that representation.

One thing this conversation is making me wonder is the extent to which we want the XDR to hide representation differences. The whole point of having subtypes is to allow a single representation (eg. "bytes") to have multiple semantic interpretations (eg. "text", "wasm", etc.)

Currently such representation questions are exposed in the XDR as something the user gets to see / is forced to care about. An alternative (as you seem to be suggesting here) is that the difference gets buried in type repertoire of the Env interface. As an illustrative example of this: the value type currently has symbol as a separate type from bytes (which we're intending to enhance with a text subtype). This has some advantages (the user has a clear sense of which XDR representations will map to which sorts of cheaper-or-more-expensive runtime representations) but also disadvantages (the user has to care about the boundary between 10 character strings of the form a-zA-Z0-9 and all other strings; they might prefer to just let the runtime treat the special case as an invisible optimization).

We could theoretically lean hard into "hide representation questions from users", possibly even making the idea of subtypes invisible to the user (eg. we have separate Value-level variants for String, Wasm, Timestamp, etc. -- each thing we treat as a mere subtype-of-some-representation-type at the runtime level).

@paulbellamy input appreciated -- this subject basically touches everything in the ecosystem any time we change it, which is why it's top of my list, should be decided ASAP.

[paulbellamy]

Is there an example of how subtypes would work in the XDR?

[graydon]

Is there an example of how subtypes would work in the XDR?

What I was hinting at above is there's kinda two separate ways to approach it. One is to keep things "factored" and put the subtype values as explicit fields in each union case. So in today's code this version looks like:

union SCObject switch (SCObjectType type)
{
   ...
   case SCO_BYTES:
      struct {
          SCOBytesSubtype subtype;
          opaque bin<SCVAL_LIMIT>;
      }
   ...
}

And then the other way is to just duplicate the union-arm for every case we'd have a subtype for, so we get:

union SCObject switch (SCObjectType type)
{
   ...
   case SCO_BYTES_TEXT:
          opaque bin<SCVAL_LIMIT>;
   case SCO_BYTES_WASM:
          opaque bin<SCVAL_LIMIT>;
   case SCO_BYTES_HASH:
          opaque bin<SCVAL_LIMIT>;
   case SCO_BYTES_WASM:
          opaque bin<SCVAL_LIMIT>;
   ....
}

This has advantages and disadvantages. The main advantage is conceptual simplicity -- if we do this with your suggestion of collapsing object and value, there's still just one conceptual level of type information in the union, which makes life simpler for users. A secondary advantage is it's smaller in terms of encoded XDR. XDR will chew up 32 bits for the union switch value no matter what -- it does no bit-packing -- so we might as well use that bit-space to its fullest extent.

I think there are two disadvantages and maybe we can live with them. The XDR definition gets potentially a bit longer looking due to all the duplication. And future extension winds up having "subtypes" somewhat out-of-order in the list -- like if we add a subtype to a union-arm that's conceptually in the middle of the list, we have to extend it with the last-assigned enum value (unless we do the thing your PR does with reserving some space after each code.

Maybe both of these are acceptable though. For example, if we lose 1 extra character from Symbol and make the value-tag space 7 bits (so it fits in a single WASM LEB128 byte) then we have 128 codes to assign; we could either "space out" values by giving 4 bits of primary value code-space (16 options) and 3 bits of subtype (8 subtypes) reserved on each. Or we could just forget about the spacing/ordering of codes (nobody's likely to care) and allocate those 128 codes "flat" to each type-or-subtype as they're added, only differentiate types-vs-subtypes by grouping various types together at the env interface layer.

(The point of subtypes existing in the first place is that some types have special handling that's different enough to warrant being described a special way -- eg. formatting text for display or such -- but are still representationally identical to their unspecialized variants, and should be allowed as arguments to all the unspecialized functions, eg. the bytes-length function should tell you the length of any bytes object, whatever flavor)

[graydon]

(Also worth noting: if we split the Env-interface tag space from the SCVal tag code space -- say to allow hiding all the Env-optimized forms like static slices and maybe even symbols from the user -- we'll still need to reserve those values from the SCVal tag code space. So .. it's likely to look a little weird there anyways.)

[graydon]

(Also worth noting: SCContractCode already has essentially a "subtype" split-out of it. So if we flattened subtypes into the main tag space we'd probably eliminate the nesting there too, put the SCContractCodeType variants into SCValType. Which would actually be possibly advantageous since SCCONTRACT_CODE_TOKEN is a void union arm -- it wouldn't require passing or allocating a host object at all.)

[graydon]

After discussion this morning we've decided to go with (or at least try going with) a version of this. And actually go quite a bit further in overhauling things:

• Collapsing the Object and Value types at the XDR level as initially suggested in this PR (the distinction will remain in the Host and a bit of the Env interface, at least insofar as the Env interface will still bit-pack some values directly but refer to others by handle)
• Removing some special cases that don't need to be visible --symbol and u63 -- at this level, reserving conceptual and tag-number space for them in the Env but not revealing them in XDR. The XDR contains abstract values only, not special cases of representation.
• Collapsing / replicating any sort of subtypes we either have now or are soon to add into this same value-space, including those used for constant slices.
• (Not discussed in meeting but actually fairly obviously wanted from public feedback and the solang experiment: adding u256/i256)

So I spent some of the afternoon sketching the result of all of this, and came up with something roughly like so:

// We fix a maximum of 128 value types in the system for two reasons: we want to
// keep the codes relatively small (<= 8 bits) when bit-packing values into a
// u64 at the environment interface level, so that we keep many bits for
// payloads (small strings, small numeric values, slices, object handles); and
// then we actually want to go one step further and ensure (for code-size) that
// our codes fit in a single ULEB128-code byte, which means we can only use 7
// bits.
//
// We also reserve several type codes from this space because we want to _reuse_
// the SCValType codes at the environment interface level (or at least not
// exceed its number-space) but there are more types at that level, assigned to
// optimizations/special case representations of values abstract at this level.

enum SCValType
{
    SCV_BOOL = 0,
    // Code 1 reserved: at the environment level we treat bool true and
    // false as separate _types_, with false=0, true=1, because these coincide
    // with the u32 values WASM uses for bools, and it reduces codesize.

    // The null/void/unit/none type: indicates the absence of other data.
    SCV_NULL = 2,

    // 32 bits is the smallest type in WASM or XDR; no need for u8/u16.
    SCV_U32 = 3,
    SCV_I32 = 4,

    // 64 bits is naturally supported by both WASM and XDR also.
    SCV_U64 = 5,
    SCV_I64 = 6,
    SCV_U64_TIMEPOINT = 7,
    SCV_U64_DURATION = 8,

    // Codes 9, 10 reserved for small-value versions of u64/i64.

    // 128 bits is naturally supported by Rust and we use it for Soroban
    // fixed-point arithmetic prices / balances / similar "quantities".
    // These are represented in XDR as a pair of 2 u64s, unlike {u,i}256
    // which is represented as an array of 32 bytes. 
    SCV_U128 = 11,
    SCV_I128 = 12,

    // Codes 13, 14 are reserved for small-value versions of u128/i128

    // 256 bits is the size of sha256 output, ed25519 keys, and the EVM machine
    // word, so for interop use we include this even though it requires a small
    // amount of Rust guest and/or host library code.
    SCV_U256 = 15,
    SCV_I256 = 16,

    // Codes 17, 18 reserved for small-value versions of u256/i256

    // TODO: possibly allocate subtypes of i64, i128 and/or u256 for
    // fixed-precision with a specific number of decimals.

    SCV_BYTES_DATA = 19,
    SCV_BYTES_TEXT = 20,
    SCV_BYTES_WASM_V0 = 21,
    SCV_BYTES_XDR_SCVAL_V0 = 22,

    // Codes 23, 24 reserved for small-array immediate SCV_BYTES_DATA and TEXT 
    // (what was previously "symbol")

    // Codes 25, 26, 27, 28 reserved for constant-slice versions of SCV_BYTES_*,
    // specified by a [contractID:12][offset:22][length:22] = 56 bit payload.

    SCV_ERROR_SYSTEM = 29,
    SCV_ERROR_CUSTOM = 30,

    SCV_VEC = 31,
    SCV_MAP = 32,

    // Codes 33, 34 reserved for constant-slice versions of SCV_VEC and MAP.

    SCV_CONTRACT_CODE_REF_WASM_V0 = 35,
    SCV_CONTRACT_CODE_TOKEN_V0 = 36,

    SCV_ACCOUNT_ID = 37,

    SCV_LEDGER_KEY_CONTRACT_CODE = 38,
};

This is not a final proposal, just a step towards one to illustrate the collective set of ideas. I'm actually fairly pleased with the way this decouples representation optimizations from abstract values; for example we can do small-object and constant-slice optimization on many different types.

Feedback welcome!

Especially on the question of whether we ought to subtype the fixed-point-7-decimals or ERC-like 18-decimals cases (there's even potentially enough codespace here to chew up a pile of codes just specifying 0..20 decimals, though I tend to think this is pointless). The recent emphasis on the SAC suggests that the i128-with-arbitrary-decimals design-point might itself not be perfect; stellar assets having 64-bits-with-7-decimals quantities everywhere.

[paulbellamy]

Overall, looks pretty nice.

Why did you include one for SCV_LEDGER_KEY_CONTRACT_CODE = 38. Isn't that a separate ledger key thing?

Edit: And actually, same question about SCV_CONTRACT_CODE_REF_WASM_V0 and SCV_CONTRACT_CODE_TOKEN_V0 as well.

Edit edit: Looked at the code, and we do have an scval type for that already, don't we...

[jayz22]

A few minor questions:

1. What does V0 stand for, for entries such as SCV_BYTES_WASM_V0 and SCV_CONTRACT_CODE_TOKEN_V0?
2. When you say codes are reserved, e.g. "reserved for small-value versions of u64/i64", are they intended to exist from day 1 but just not exposed by xdr. Or will they not exist and are intended to be added in the future?
3. All the types that are previously defined under Object, part of their encoding still include an object handle? This relationship is no longer obvious from the xdr. I wonder if it's still worthwhile to point out such connections (for clarity purposes), or completely hide it at the xdr level.
4. What are the intentions for the two error types SCV_ERROR_SYSTEM and SCV_ERROR_CUSTOM? Are they intended to cover all error codes under current ScStatus?

[deian]

This looks great. One thing (sorry if this is obvious): Is the string encoding fixed? This might be less relevant at the XDR level than at the SDK level.

[graydon]

@jayz22

1. The V0 just saves us from having to come up with some other term if-and-when we add a new incompatible version of either of the referenced types. I could leave it off for now and put _V1 or _V2 on future versions only. Don't care.
2. They're "reserved" in the sense that they live at the env level but not the XDR level. We want the entire code-space at the env level to be less than 128 values (so it fits in a ULEB128 byte), and codes at the XDR level are going to be "some subset of the codes at the env level", and so the easiest way to ensure that subset-relationship holds is to just .. spell out all the codes that are present at the env level, in the spaces between the XDR-level codes.
3. The XDR encoding didn't mention object handles before and it still doesn't here. SCObject types in the XDR always appeared to just be contained-within their SCVal -- they were split into a handle-containing RawVal and a HostObject when lowered to the env level. Same thing will happen here (behind option types when they're recursive).
4. Yes the error codes are supposed to go in there. There's no need to fight over space in the tag-space anymore (we have lots) so user and system errors can be differentiated at the outer tag level and we can keep the whole inner major/minor space for each.

@deian I was planning on going with UTF-8 here but @MonsieurNicolas has mentioned in the past it might make sense to include a built-in case for a different encoding (and of course such subtyping is just a convenience for display -- you can always put any encoding you want into a plain BYTES_DATA object)

[paulbellamy]

Closed in favor of #70