Improving Storage Migration with Changes to map_new_from_slices, map_unpack_to_slice, and contracttype

[leighmcculloch]

Data migration in Soroban contracts is currently more difficult than it needs to be. When a contract's data structures evolve—such as adding new optional fields—developers face significant friction.

Consider a simple upgrade scenario: a contract stores DataV1 with fields {a, b}, and needs to upgrade to a new wasm where that data will have a new field c and become {a, b, c}.

Examples

Example 1: Adding an Optional Field

I expect a developer's natural assumption would be to try using an Option<T> field. It's easy to assume based off the behaviour of many other APIs that data without the field would deserialize with c = Option::None:

#[contracttype]
pub struct DataV1 { a: i64, b: i64 }

#[contracttype]
pub struct DataV2 { a: i64, b: i64, c: Option<i64> }

#[contracttype]
pub enum DataKey {
    Data(u32),  // keyed by id
}

// Developer expects this to work when reading old V1 data:
let data: DataV2 = env.storage().persistent().get(&DataKey::Data(id)).unwrap();
// Expected: c would be None for old entries
// Actual: TRAPS with Error(Object, UnexpectedSize)

This fails because the host validates field count and names before the SDK or the developer can handle any mismatch.

Example 2: Explicit Fallback Logic

Knowing the simple approach doesn't work, a developer might try explicit fallback with try_from_val. The name suggests you can try to convert from the val, and in the vast majority of implementations try_from_val results in a guest side error if it fails. However that's not the case with contracttype structs.

#[contracttype]
pub struct DataV2 { a: i64, b: i64, c: i64 }

let raw: Val = env.storage().persistent().get(&DataKey::Data(id)).unwrap();

// This also TRAPS instead of returning Err!
if let Ok(v2) = DataV2::try_from_val(&env, &raw) {
    v2
} else {
    // Never reached - host traps before try_from_val returns
    let v1 = DataV1::try_from_val(&env, &raw).unwrap();
    DataV2 { a: v1.a, b: v1.b, c: 0 }
}

The host traps with Error(Object, UnexpectedSize) before try_from_val can return an Err.

Current Workarounds

Workaround 1: Version Marker

The best solution to work around this that I'm aware of today is to use an explicit version number and check it before reading.

#[contracttype]
pub enum DataKey {
    DataVersion(u32),  // version marker keyed by id
    Data(u32),         // data keyed by id
}

fn read_data(env: &Env, id: u32) -> DataV2 {
    let version: u32 = env.storage().persistent().get(&DataKey::DataVersion(id)).unwrap_or(1);
    match version {
        1 => {
            let v1: DataV1 = env.storage().persistent().get(&DataKey::Data(id)).unwrap();
            DataV2 { a: v1.a, b: v1.b, c: None }
        }
        _ => env.storage().persistent().get(&DataKey::Data(id)).unwrap(),
    }
}

This works but discovering this pattern is non-obvious.

This can be retrofitted on to any existing storage because the program can assume that if the version doesn't exist that it is the first version of the data.

Workaround 2: Map Inspection

I have also seen LLMs suggest work arounds to inspect the map, either its length or the symbols in the map, both both approaches are either very verbose or error prone. For example:

// Read as raw Map to inspect structure
let raw: Map<Symbol, Val> = env.storage().persistent().get(&DataKey::Data(id)).unwrap();

if raw.len() == 3 {
    // V2 format
    env.storage().persistent().get(&DataKey::Data(id)).unwrap()
} else {
    // V1 format - read again and convert
    let v1: DataV1 = env.storage().persistent().get(&DataKey::Data(id)).unwrap();
    DataV2 { a: v1.a, b: v1.b, c: None }
}

Proposal

Introduce v2 versions of the map packing/unpacking host functions with the following semantics:

1. map_unpack_to_slice_v2: When reading maps:
   a. Unpack missing keys as Val::VOID values (which convert to None for Option<T> fields).
   b. Ignore keys not requested if they are in the map being unpacked.
   c. Return an Error instead of trapping on schema mismatches.

2. map_new_from_slices_v2: When writing maps, omit key-value pairs containing 'void' values, for the main purpose of symmetry with the unpack host fn, although as a bonus as an optimisation (originally proposed in https://github.com/orgs/stellar/discussions/1750).

These changes would support a developer migrating using patterns like:

#[contracttype]
pub struct DataV1 { a: i64, b: i64 }

#[contracttype]
pub struct DataV2 { a: i64, b: i64, c: Option<i64> }

#[contracttype]
pub enum DataKey {
    Data(u32),  // keyed by id
}

// Developer expects this to work when reading old V1 data:
let data: DataV2 = env.storage().persistent().get(&DataKey::Data(id)).unwrap();

Why This Must Be Solved at the Host Level

Handling schema mismatches guest-side is impractical because validating all the fields would require iterating over the map in full and nullify any benefit of using the map unpack host fn. While in some cases a developer might optimise and only check the field count that is an incomplete check in the cases that fields have been removed or renamed.

Alternatives

1. Improve Documentation

There exists very little documentation for how to do migrations on Soroban, so I've opened an issue to improve the documentation about migrations, and if everyone needing to do migrations read that documentation, then that might suffice:

• Add how-to guide for storage data migration stellar-docs#2228

Related

• Original discussion on this topic (optimization-focused): #1750
• Test vector demonstrating migration difficulty: stellar/rs-soroban-sdk#1715

[dmkozh]

This makes sense to me. The behavior might be slightly surprising if someone tried to inspect the map manually, but we don't expect this to be a common pattern (even though LLMs suggest this currently). But I wonder if we should name this more appropriately to the use case, like unpack_contract_type_to_slice, contract_type_new_from_slices. Contract type spec is already a de facto part of the protocol and I don't see much value in obfuscating these functions (unless we believe that there are use cases for them beyond contract types, that would make names confusing).

[leighmcculloch]

if someone tried to inspect the map manually, but we don't expect this to be a common pattern

Inspecting the maps is common and easily done through several tools, such as the stellar-cli, and the lab. But I don't think this will be surprising. Omitted fields often make APIs less clear as to the full shape of an object, but it is such a common practice in APIs to omit empty, null, or none fields so I think that's fine.

unpack_contract_type_to_slice, contract_type_new_from_slices

I think the map names are more suitable. These functions are for operating on host maps, and contract type's are just one user-land use case. Contract event data bodies are a second use case that sort of look like contract types but the structs don't actually work the same way on the guest side. Another use case that was imagined at one point was us adding bulk map pack and unpack operations accessible to users, although that never eventuated.

[dmkozh]

Inspecting the maps is common and easily done through several tools, such as the stellar-cli, and the lab

I mean inspecting/interacting with the maps in code. For off-chain uses that's fine (as these just look at the storage without making assumptions about the implementation). But in code if I put ((key, None)) into map, I would expect that map.has(key) == true, which is not going to be the case here. That's why I'm not sure if it's a good idea to expose these functions as generic helpers - I feel like it might lead to footguns if developers don't read the docs. Specialized helpers would likely reduce the risks. We can call this 'sparse_map' or 'lazy_map' to keep things relatively generic. Basically I'd like the name to imply that this does not simply put the provided key-val pairs into map.

[leighmcculloch]

Ah, yes that's a really great point.

sparse_map

👍🏻 Naming them sparse_map_new_from_slices and sparse_map_unpack_to_slice would better communicate the behaviour.

To tell the narrative better, the SDK could have a SparseMap type that parallels the behaviour of these functions. Another option is a new type of Host type called SparseMap, and these new functions would be for unpacking and packing SparseMaps. That would create a single vertical where these semantics are true rather than creating different semantics at different layers. Similar to how Address can be upgraded to MuxedAddress, we could make it so that SparseMap can be converted into a Map.

In practice would anything use this other Host type? Unclear, I don't think so, so probably not worth it. The question for me is, is this change in behaviour significant enough to warrant a type incompatibility where one is not the other?

[dmkozh]

I think introducing a new host type is a bit of an overkill: it's a lot of work on our side without a clear benefit, and also it wouldn't be compatible with any existing events. Unlike the MuxedAddress case where we conveniently could modify the underlying ScAddress of ScVal::Address, this would require an explicit ScVal variant. It also would make migration to the sparse map format harder for the existing contracts. Thus I think using the more clear host fn names in combination with the existing map type is a reasonable tradeoff.

I also agree that it seems unlikely that the new host type would be too useful outside the SDK machinery.

[leighmcculloch]

Agreed, I don't think the difference in behaviour is significant enough to warrant type incompatibility.

[leighmcculloch]

@khomiakmaxim posted at stellar/stellar-docs#2228 (comment) highlighting that the enum data value pattern provides another workaround as a way to introduce multiple versions of types without the need for a separate version storage. It requires thinking ahead to add that for every data value, but for anyone who has done that there's an avenue to adding other types. It's a bit cumbersome to use for every type since every time you use the value it has to be unpacked, but it would work.

[mootz12]

@leighmcculloch This proposal might also be useful to support storage migrations -> https://github.com/orgs/stellar/discussions/1932

[dmkozh]

I've created CAP-86 that implements the proposal here.

[leighmcculloch]

1. b. Ignore keys not requested if they are in the map being unpacked.

In the proposal at the top I mention this feature which is included in CAP-86@597d215, but reflecting on the examples in CAP-86 (that were really good, thanks @dmkozh!), I think it has a downside and isn't strictly required to support the Option<> usage.

Ignoring keys becomes very easy to include fields in input data to functions that get ignored. It makes it easy to typo a field name. Say during the migration a new field is added with Option<> type, and ahead of the new field being added calling contracts start specifying the new field but due to some miscommunication the wrong name is used. Even after the migration the contracts will continue to pass the field that gets ignored.

An upside of 1b, that I don't think we intend, is it significantly increases the flexibility of calling a variety of contracts that implement the same API but with different extensions supporting additional fields that are safe to ignore.

If no one sees a significant benefit with retaining 1b, I think we should retract that part of the proposal. It would mean that all stored/passed fields must be decoded, but missing fields are okay and decode as Option::None.

Keen to hear what others think of this.

[leighmcculloch]

If we retract 1b today and it later becomes of interest, it'll be possible to add it later too.

[tupui]

I think both can work and depends more on how you communicate what get does. Like select in SQL basically. There it's clear that you can be explicit to pull whatever you need. Would there also be any performance impact?

[dmkozh]

The motivation behind the CAP is really to make the data structures more compatible with each other, so I think in general it's fine to have behavior that goes beyond just better option support. But I agree that it's worth revisiting the benefits and downsides of the approach.

Ignoring keys becomes very easy to include fields in input data to functions that get ignored.

That's actually precisely the reason for why I think this might be valuable in the first place, as it allows extension of the caller API without updating the callee (which is something discussed in https://github.com/orgs/stellar/discussions/1943#discussioncomment-17479468 and in one of the examples in CAP). It is indeed not strictly mandatory, as the pattern can be inverted to update the callee first, but that's still something with a plausible use case that might be more convenient in some cases.

Say during the migration a new field is added with Option<> type, and ahead of the new field being added calling contracts start specifying the new field but due to some miscommunication the wrong name is used.

That's true, but on the other hand the upside is that if a field gets removed from the struct definition during the storage schema migration, we won't necessarily brick the contract. Also, in this scenario it would make more sense for callers to add the field as non-optional, so when the callee tries to get the incorrectly named non-optional field, they will fail on Void->non-optional conversion.

An upside of 1b, that I don't think we intend, is it significantly increases the flexibility of calling a variety of contracts that implement the same API but with different extensions supporting additional fields that are safe to ignore.

Yeah, that's an upside, and I think it actually might be quite useful. It also allows for cleaning up the deprecated fields.

If we retract 1b today and it later becomes of interest, it'll be possible to add it later too.

That would be a bit sketchy TBH, as we'd probably need the third version of the conversion functions.

There it's clear that you can be explicit to pull whatever you need. Would there also be any performance impact?

There is no significant performance difference between the approaches, it's just about the right semantics. The current proposal is not exactly like SELECT, because you can actually fetch a field that doesn't exist in the record (and get Val::Void/Option::None back). There is basically no schema for any given record, and every caller may have its own schema definition.

In the end, I don't believe that 1b behavior is strictly necessary, but I find it rather consistent with the rest of the changes - it allows for more intuitive data migration scenarios (regardless of the 'direction' of the migration). If we don't go for this, developers will still need to sometimes rely on the less intuitive patterns (but of course much less counter-intuitive than the current patterns).

[dmkozh]

Basically, with the current proposal we are putting the responsibility for figuring out which fields are required on the reader only. Without 1b the responsibility will be partially on reader and partially on writer (because if writer writes a field, every reader will have to consume it). Personally, I believe that reader-only-responsibility approach is cleaner and more user-friendly.

[leighmcculloch]

All great arguments. Thanks.

I realise too that I am mostly thinking about 1b in the context of JSON APIs that frequently have no schema.

But the contract APIs we are using this in are much more akin to proto APIs which have a schema and its through schema (contractspecs) driven clients that most interactions with contracts take place. So I think the downside I described earlier are much less likely to show up in practice. In a similar way they don't show up in proto usage either.

Let's keep 1b.

[dmkozh]

In the light of revisiting our assumptions about the CAP features, I'd like to highlight one potential issue with item 2 (map_new_from_slices_v2 function).

Given that data migration story is fully covered by the item 1 (i.e. the reader function), the new map creation function is introduced purely as an optimization. It's a useful optimization in a world where every contract uses the new reader function. But in the current world, switching to the new function may cause breakage in cross-contract APIs: if API involves a contractype with an Option field (either as an argument, or as a return value), then if only one contract is updated, the second contract will trap if that Option field is None. This is harder to catch than an unintended interaction with contract's own storage.

We can still introduce the function and make the decision later at the SDK level. Maybe the optimization is only useful for the events (where there is no API incompatibility risk). But also maybe my concern is unwarranted and UDT-based APIs are rare, then we don't really need to worry. Some additional data analysis is necessary.

Any thoughts? @leighmcculloch

[dmkozh]

I was meaning updating both. The only break I see is that subtle breakage you note, or the subtle change to a field not being present and visible if someone reads it with the Map type.

So just to be clear, are you voting for the replacement instead of new fns? I'll try brainstorming possible failures a bit more, but for now that seems fine to me.

[leighmcculloch]

After pondering this more, I think the safest thing tied to a real use case is a new unpack fn only and abandon the optimisation. I think the unpack alone is safe because it's very unlikely a contract was relying on lack/presence of a field to drive some panic path. If only doing unpack we don't win anything by modifying the existing fns.

Everything else seems like a risk that don't round to zero.

I'm still open to other thoughts on this for how we could make a pack-and-unpack implementation work either as new fns or replacement fns. But in lieu of that, lets move forward now with the new sparse unpack fn only.

[dmkozh]

I still don't see much harm in keeping the packing function, as we could make it explicitly opt-in at the SDK level, so that it could be used for e.g. events. With explicit opt-in the risk of the breakage I've described seems pretty low. I don't mind excluding it as well, but just wonder if you see any issue with explicit opt-in.

FWIW I've almost convinced myself that changing the existing functions is fine. But going with a conservative option is still a much more straightforward path forward right now.

[leighmcculloch]

keeping the packing function, as we could make it explicitly opt-in at the SDK level, so that it could be used for e.g. events. With explicit opt-in the risk of the breakage I've described seems pretty low. I don't mind excluding it as well, but just wonder if you see any issue with explicit opt-in.

I think it's fine to keep it in the host changes, and we can explore and test out ideas in the SDK. The worse case is we don't end up using it in the short term.

FWIW I've almost convinced myself that changing the existing functions is fine.

Can you elaborate on this?

[dmkozh]

Can you elaborate on this?

I just mean that it seems unlikely for the behavior change to be observable by the sane contracts, whereas contracts that do something weird (inspect the map) may still be broken by the SDK reader change (though admittedly that's going to be much easier to catch). But really let's not spend more time on this - we can go with the CAP as is, experiment with it on the SDK side, and then we can come back to the old function migration topic if necessary.

[MonsieurNicolas]

Should there be some work (SDKs, linters etc) so that writers keep track of previously used fields and/or discourage complete removal of deprecated fields?

It seems hard to guarantee that there are no readers and/or data that exists (on or off chain) with the old schema if there is no version number.

In the CAP, the example shows that the "V3" version is

#[contracttype]
struct AdminInfo {
    admins: Vec<Address>,
    is_frozen: bool,
}

instead it should keep the sentinel admin from the previous version (the type does not matter).

By removing the field entirely, as there is no version numbers, a potential footgun is armed where someone could reintroduce the same name but for a different use case.

It may sound like an edge-case, but edge cases are exactly how we end up with security issues, in this case under the "type confusion" class of problems.

[dmkozh]

By removing the field entirely, as there is no version numbers, a potential footgun is armed where someone could reintroduce the same name but for a different use case.

FWIW in the storage case you write the whole struct back into storage (i.e. the updates to the underlying map are overwrites, not merges). So as soon as you write a new version of AdminInfo to storage, you'll automatically get rid of the old contents, and the old field names will end up re-usable. Off-chain this shouldn't be an issue as well, unless assumptions are made based on the outdated storage case.

I agree that more care is necessary in case of cross-contract APIs, and field removal should be discouraged for that use case. We can of course be more conservative and just discourage field removal in general, but I think there may be valid cases for that even without any versioning. Also the new map writer function would help for the cases where field removal is not desirable - the schema can maintain all the fields forever as Void, while the data itself would only contain what's necessary.

[leighmcculloch]

discourage complete removal of deprecated fields

as soon as you write a new version of AdminInfo to storage, you'll automatically get rid of the old contents

It really depends on the use case I think.

Something like a singleton AdminInfo would be pretty safe to drop the field.

In a successful contract with data spanning many users data migration will have a long horizon. If much of that data is archived then data migration might never complete in a migrate-data-as-it-is-used approach. As a result it'd be safer to assume that fields should just generally never be removed and migration will live on.

The changes proposed at the top and captured in the CAP should support both use cases and others.

If the field can be removed the best practice could be to leave a residual comment rather than keep the field.

SDKs, linters etc so that writers keep track of previously used fields

We can't check these types of behaviours in the SDK, but there's room for other tools such as a lint to hint best practices like this to developers. Rust already has static analysis tools like cargo-semver that will check if a change to a type results in a breakage, so something like that but focused on Stellar/Soroban specifics.

But I think we should start with docs and examples describing what the data migration best practices are, then tools can build off that and it will teach agents the practices too. @mootz12 and I have been discussing the need for material about data migrations and were planning a technical talk about it for later this year, but we could start with docs.

[leighmcculloch]

The SDK change to use the new unpack host function is here:

• Unpack contracttype structs sparsely (CAP-86) rs-soroban-sdk#2006

There should be little surprising in this change. It replaces use of map_unpack_to_slice on struct contracttype types to sparse_map_unpack_to_slice.

[leighmcculloch]

The SDK change to use the new pack host function for events only is here:

• Pack contractevent (data format = map) sparsely (CAP-86) rs-soroban-sdk#2008

As a result of the conversation that occurred at https://github.com/orgs/stellar/discussions/1877#discussioncomment-17651313 I am only replacing use of map_new_from_slices with sparse_map_new_from_slices for contract events only, and not for all contract types as originally planned when this idea was first discussed in #1750. Contract types will always pack all their fields because to do otherwise would affect cross contract compatibility. Events are not consumed on chain and so the same risk doesn't exist.

Feedback appreciated!