stellar / rs-soroban-sdk Goto Github PK

We should review in greater detail the Ethereum contract metadata for inspiration for the types of things people like to store alongside contracts.

https://docs.soliditylang.org/en/v0.8.14/metadata.html#encoding-of-the-metadata-hash-in-the-bytecode

Idea 2:

• Have the SDK touch all components of incoming values.
• For Vecs, Maps, and Sets, this would involve fully iterating the collection.

Originally posted by @leighmcculloch in #152 (comment)

After the recent reorg #70 the size of the .wasm files are big. For example the add example is 12kb.

There could be a couple explanations:

1. Something is pulling in a large piece of the std crate. It turned out we were doing this with String I'm the env crate, but fixing that didn't fix the problem.

2. The xdr crate could be to blame. This occurred also around the same time we started using the xdr crate in the env package beyond tests. The xdr package is pretty big with a large number of types. We don't use most of them.

3. Workspace feature selection is to blame rust-lang/cargo#10636 and we might be pulling in something that we didn't plan to.

Sub-areas:

• #53
• #52
• stellar/rs-soroban-env#162
• #166
• stellar/quickstart#351
• #47
• #178

This description is updated with the resolution:

The design is here: #42 (comment)

The list of things to do now are here (#42 (comment)):

• #114
• #115
• #116
• #24
• #117
• #153
• https://github.com/stellar/rs-stellar-contract-sdk/issues/168
• #118
• stellar/soroban-cli#6
• stellar/soroban-cli#7
• stellar/soroban-cli#8
• #142

The change stellar/rs-soroban-env#120 renamed host functions. This is fine and we're still iterating and I'm not upset about it but it broke existing wasm blobs (eg. those embedded in tests) that expected the old host function names to exist.

This revealed / reveals a thing I think we should probably encode in contracts: the host function API version they were compiled against. Not because I expect we'll multi-version the API surface or anything -- I expect long-term we'll commit to a strictly backward-compatible API unless there are dangerous bugs -- but because I think @jonjove spent a while hunting the bug and it would be better to give concise diagnostics for such mismatches (new contract running on old host, or compatibility break for whatever reason) during contract instantiation rather than failing inscrutably during execution.

So I think we should have "the version of the host you were compiled against" as metadata pasted into yet another custom section in the wasm (or in the spec section if you can include it there, I don't care).

The contract spec macros will be able to generate traits and trait impls for calling other contracts, but we should have this also generate a trait and trait impl for its own implementation to validate that it can build it successfully. @graydon suggested this here: https://discord.com/channels/897514728459468821/984210243770789999/988839624652754986

This would identify issues where a user defines their own UDT named "Symbol" and the macro identifies that as a sdk::Symbol rather than a udt::Symbol.

Cloudflare Workers have a Rust SDK for interacting with their products from Rust built to WASM and running in V8. There might be things to learn from that SDK. We should review the SDK.

https://blog.cloudflare.com/workers-rust-sdk/

Problem

In some cases it is impractical for a contract developer to know when they should fully decode a type or not because the Rust SDK makes it seems like types are always concrete and safe.

For example, if a developer receives as input to their function a Vec<i32>, it may not be obvious that the types in the Vec are not guaranteed to successfully decode to i32's. For example, the Vec's fifth element could be an i64, and with the existing as it is written today the Vec would panic/trap when attempting to read the fifth element.

This could be a nasty footgun. The developer might store the Vec without inspecting its elements, and then later attempt to iterate it, which would fail. This could indefinitely brake a contract.

Coming up with a clear story for how developers should safely use and validate inputs is required to ensure developers write safe contracts that do not make assumptions about user input.

Goal

The goal is to make the footgun with our collection types less of a footgun.

Realtime Discussion Thread

https://discord.com/channels/897514728459468821/989953055829135380

Consulted

This issue needs consultation from the following people at minimum:

@graydon @leighmcculloch @MonsieurNicolas @jonjove @paulbellamy

Experiments

• #165
• #188
• #177
• #209

Cargo features are limited to being additive only and if multiple dependencies in the dependency tree depend on a shared dependency, and require different features, those feature sets get added together when building the dependency.

This is often not a problem, except that it is for the sdk and its use of the stellar-xdr crate.

The stellar-xdr crate has a std feature that if enabled pulls in types from the Rust std::io module. It uses these types for serializing/deserializing XDR types to byte buffers / files / etc.

The stellar-contract-macros crate (in this repo) is dependent on the stellar-xdr crate with the std feature enabled, because the macros serialize type information into the contract spec XDR.

The stellar-contract-env-common/guest and stellar-contract-sdk crates are dependent on the stellar-xdr crate without the std feature enabled, because our contracts disallow the use of std to keep their file size low.

Due to the fact that dependencies of macros pollute the dependency tree of regular dependencies, even though they are only a build-dependency, and due to cargo's additive features and the stellar-xdr crate gets compiled once with std enabled. To combat this we currently cause (initially it was by accident) cargo into compiling two versions of stellar-xdr, one with std enabled and one without for each use case. The way we do this is by specifying that the stellar-contract-macros crate is dependent on a different commit of stellar-xdr.

This current hack that makes it work is not ideal. In any build of the SDK and the environment the same version of stellar-xdr should be being used in all components.

cc @graydon
fyi @jonjove @paulbellamy who may upgrade dependencies in the sdk

To support writing tests it is important that the Env expose in tests functions for setting up ledger state, as well as have the ability to inspect the state after some contract invocations have occurred.

A primitive / prototype form of this functionality existed in an early version of the SDK before more serious development and integration with stellar-core began, and was lost it when we started integrating it with stellar-core, because the prototype was not compatible with stellar-core's ledger state.

We probably need to take a step steps towards this:

• #167
• #179
• stellar/rs-soroban-env#491
• #181

Fuzzing:

• https://github.com/AltSysrq/proptest (Already experimented with in #99 #53)
• https://github.com/rust-fuzz/afl.rs
• https://github.com/rust-fuzz/cargo-fuzz
• https://github.com/loiclec/fuzzcheck-rs

I spoke to @nano-o today who suggested we also look into:

• Symbolic execution

  • https://crux.galois.com/ (Rust)
  • https://www.cprover.org/cbmc/ (C)
  • https://github.com/trailofbits/manticore (supports WASM)

• Tools that test for reentry bugs

Depending on the outcome of this conversation: https://discord.com/channels/897514728459468821/935999748840751125/994771043602407454

Related to #191 #189

Depending on the outcome of this conversation: https://discord.com/channels/897514728459468821/935999748840751125/994771043602407454

Dependent on stellar/rs-soroban-env#180 and stellar/rs-soroban-env#183

Related to #189

What problem does your feature solve?

Right now, doing math can necessitate many clones.

What would you like to see?

Something like

impl<'a> Add for &'a BigInt {
    type Output = BigInt;
    fn add(self, rhs: Self) -> Self::Output {
        // ...
    }
}

but for every operator.

What alternatives are there?

Continue using clone.

We should add support for user defined enum types that contain different RawVals.

In CAP-52 @jonjove demonstrates a contract that receives an enum value that could be an AccountId or a ContractId. It is highly likely contracts will want to support receiving arguments that are one-of a series of types.

Contracts could implement this by having multiple functions where one function accepts one type, and another accepts another type. This does not scale well though once you start having multiple arguments that each have a one-of requirement.

We should make it easy in the Rust code to go from a RawVal to a user defined enum type where the enum type has arms that are all types that map from RawVals. It could be something like:

pub enum Id {
    AccountId(sdk::AccountId),
    ContractId(sdk::ContractId),
}

pub fn contract_fn_1(from: RawVal, to: RawVal) {
    let from: Id = from.try_into().unwrap();
    let to: Id = to.try_into().unwrap();
    match from {
        Id::AccountId(aid) => ...,
        Id::ContractId(cid) => ...,
    }
}

We should add support for smooth conversions between tuples and RawVals.

In the protocol meeting today @jonjove talked about how he wanted to have a tuple with a small number of values and using the Vec type felt like a lot of complexity and boilerplate. We should be able to make tuples convert to and from a RawVal, using a vec under the hood, but where the user doesn't have to deal with those specifics.

This is particularly worth doing because tuples are often different typed values, and Vec's lend themselves to a list of the same types of values.

What problem does your feature solve?

There is an issue around not being able to just unwrap certain errors, requiring a map_err first. See #149 (comment).

What would you like to see?

unwrap or expect "just work".

What alternatives are there?

None.

Idea 4:

• Make collection types always return Result's for their accessors, so it is clear that getting a value can fail if the type isn't of the expected type.

Originally posted by @leighmcculloch in #152 (comment)

Given a wasm file path, or a contract ID and a Horizon URL, generate contract calling code and testing capabilities.