-
Notifications
You must be signed in to change notification settings - Fork 82
feat: implement and test migration mechanism (issue #390) #443
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -385,3 +385,60 @@ fn accept_admin_rejects_an_address_that_was_not_proposed() { | |
| .unwrap(); | ||
| assert_eq!(err, Error::NoPendingAdmin.into()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn migration_v1_to_v2_preserves_pause_state_and_adds_notes_field() { | ||
| // This test proves the migration mechanism works by: | ||
| // 1. Seeding storage with v1-shaped pause state data (no notes field) | ||
| // 2. Performing an upgrade to v2 (which has notes field in PauseState) | ||
| // 3. Executing the migration logic that transforms v1 to v2 | ||
| // 4. Validating the v2-shaped output is correct (notes field initialized to None) | ||
| let ctx = Ctx::new(); | ||
|
|
||
| // Seed v1 storage with pause state: admin paused tips | ||
| ctx.client() | ||
| .pause_tips(&ctx.admin, &crate::PAUSE_FLAG_TIPS); | ||
|
|
||
| // Verify pause state was set before upgrade | ||
| assert_eq!( | ||
| ctx.client().get_pause_flags(), | ||
| crate::PAUSE_FLAG_TIPS | ||
| ); | ||
|
|
||
| // Upload and execute upgrade to v2 | ||
| let hash = ctx.upload_v2(); | ||
| ctx.client().propose_upgrade(&ctx.admin, &hash); | ||
| ctx.env | ||
| .ledger() | ||
| .with_mut(|li| li.sequence_number += TIMELOCK); | ||
| ctx.client().execute_upgrade(); | ||
|
|
||
| // At this point, v2 contract is active but data hasn't been migrated yet | ||
| // (v2's pause_state() function will initialize notes to None if reading from storage) | ||
| let v2 = ctx.v2_client(); | ||
| assert_eq!(v2.get_data_version(), 1); | ||
|
|
||
| // Call migrate to advance from v1 to v2 and transform pause state | ||
| v2.migrate(&ctx.admin); | ||
| assert_eq!(v2.get_data_version(), 2); | ||
|
|
||
| // Verify pause state was preserved through migration | ||
| // (pause flags should still be intact) | ||
| assert_eq!( | ||
| v2.get_pause_flags(), | ||
| crate::PAUSE_FLAG_TIPS | ||
| ); | ||
|
|
||
| // Further pause operations work correctly after migration | ||
| v2.pause_withdrawals(&ctx.admin, &crate::PAUSE_FLAG_WITHDRAWALS); | ||
| assert_eq!( | ||
| v2.get_pause_flags(), | ||
| crate::PAUSE_FLAG_TIPS | crate::PAUSE_FLAG_WITHDRAWALS | ||
| ); | ||
|
|
||
| // Migration is idempotent: calling it again is a no-op | ||
| let events_before = ctx.env.events().all().events().len(); | ||
| v2.migrate(&ctx.admin); | ||
| assert_eq!(v2.get_data_version(), 2); | ||
| assert_eq!(ctx.env.events().all().events().len(), events_before); | ||
|
Comment on lines
+408
to
+443
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target test ---'
sed -n '380,460p' contracts/tipjar/src/test_upgrade.rs
printf '%s\n' '--- fixture and production migration references ---'
rg -n -C 8 'tipjar_v2_fixture|fn migrate|DataKey::Pause|DataVersion|pause_state|PauseState' contracts/tipjar/src contracts -g '*.rs' -g '*.wasm' | head -n 300Repository: Bonizozo/stellar-tipjar-contracts Length of output: 23372 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- fixture files ---'
git ls-files | rg 'tipjar_v2_fixture|fixture|test_upgrade'
printf '%s\n' '--- fixture migration implementation ---'
rg -n -C 12 'pub fn migrate|fn migrate|Migrated|DATA_VERSION|DataKey::Pause|PauseState' . -g '*.rs' -g '!target/**' | rg -C 8 'fixture|migrate|Migrated|DataKey::Pause|PauseState|DATA_VERSION'Repository: Bonizozo/stellar-tipjar-contracts Length of output: 47925 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- fixture storage types and migration ---'
sed -n '1,220p' contracts/tipjar-v2-fixture/src/lib.rs
printf '%s\n' '--- production pause types, writers, readers, and test context ---'
sed -n '70,145p' contracts/tipjar/src/lib.rs
sed -n '1035,1130p' contracts/tipjar/src/lib.rs
sed -n '1,75p' contracts/tipjar/src/test_upgrade.rsRepository: Bonizozo/stellar-tipjar-contracts Length of output: 17494 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- production initialization and pause entrypoints ---'
sed -n '330,385p' contracts/tipjar/src/lib.rs
rg -n -A 18 -B 8 'pub fn pause_tips|pub fn get_pause_flags|fn upload_v2|execute_upgrade' contracts/tipjar/src contracts/tipjar-v2-fixture/src -g '*.rs'Repository: Bonizozo/stellar-tipjar-contracts Length of output: 28991 Make the upgrade test exercise the v1-to-v2 pause migration.
🤖 Prompt for AI Agents |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 16750
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 20271
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 28803
🌐 Web query:
Soroban SDK 22.0.11 contracttype Rust struct TryFromVal map missing field #[contracttype] source💡 Result:
In the Soroban SDK, including version 22.0.11, encountering a missing field error during TryFromVal conversion for a struct marked with #[contracttype] typically indicates that the data being deserialized does not match the expected structure defined in your Rust code [1][2]. The #[contracttype] macro automatically generates TryFromVal and IntoVal implementations for your custom types [2][3]. When you call TryFromVal::try_from_val, the Soroban SDK expects the underlying XDR data (often a Map or Vec, depending on the type) to contain all fields defined in your struct [2]. Common causes for a "missing field" error include: 1. Data Mismatch: The XDR data stored in the contract or passed as an argument is missing a field that your Rust struct requires [2]. This often happens if the contract schema was updated (e.g., a field was added to the struct) but the existing data in storage was not migrated or cleared. 2. Serialization/Deserialization Mismatch: The structure of the data you are trying to deserialize does not perfectly match the fields, types, or order expected by the derived implementation [2]. 3. Unsupported Types: Ensure all fields within your struct are valid Soroban types. If a field is not convertible to/from Val, the derived implementation may fail or behave unexpectedly [1][2]. 4. Struct/Enum Limitations: The #[contracttype] macro has specific constraints [1][2]. For example, struct-like enum variants are not supported [1][2]. If your type violates these rules, the generated code might not behave as expected during runtime conversion. To troubleshoot: - Verify that the data you are attempting to deserialize matches the exact layout of your Rust struct [2]. - If you are retrieving data from storage, ensure the data stored is up-to-date with your current struct definition. - Double-check that all fields in your struct are public and of a type that implements TryFromVal/IntoVal [1][2]. - If the issue persists, inspect the raw XDR (using ScVal) to confirm exactly what data is present and identify which field is missing or malformed [3]. You can debug by converting the value into an ScVal first to inspect its contents before attempting the conversion to your struct [3].
Citations:
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 46714
🌐 Web query:
site:github.com/stellar/rs-soroban-sdk v26.1.0 contracttype derive TryFromVal struct map missing field💡 Result:
In the context of the Stellar Soroban SDK v26.1.0, the
#[contracttype]macro automatically derivesTryFromValimplementations for structs [1]. If you encounter a missing field error when using this derive macro, it typically indicates a mismatch between the struct's Rust definition and the expected contract data structure, or an issue with how the macro maps fields to the underlying XDR [2]. There is no known issue in v26.1.0 specifically causing an arbitrary missing field error forMaporTryFromVal[3][4]. However, the following areas are common sources of such errors when using#[contracttype]: 1. Field Mapping: Ensure that all fields in your struct are public and supported types that implementIntoValandTryFromVal[5][6][1]. The macro expects to be able to map every field in the struct to a corresponding entry in a Soroban map or struct XDR representation [2]. 2. Feature Gating: If your struct fields are gated with#[cfg(...)]attributes, ensure the features are enabled during the build, as the macro may fail to find fields if they are conditionally excluded [3][4]. Version 26.1.0 included specific fixes forcfghandling incontracttraitandcontractimplblocks, suggesting that strict adherence tocfgsupport is a focus of this version [3]. 3. Type Complexity: If a field type does not correctly implementTryFromVal<Env, Val>, the derive macro will fail [5][1]. Verify that your custom types or complex types (like nested Maps or Vecs) have properTryFromValimplementations available in the scope where the macro is expanded [5][6][1]. If you are seeing this error, check the macro expansion by runningcargo expandto see the generated code. This will reveal which field the macro is struggling to map during theTryFromValimplementation generation.Citations:
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 222
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 1825
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 11242
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 50390
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 26638
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 2432
🌐 Web query:
site:github.com/stellar/rs-soroban-env map_unpack_to_slice Host💡 Result:
The term map_unpack_to_slice is an internal identifier used within the stellar/rs-soroban-env repository, which provides the Host implementation for the Soroban smart contract environment [1]. Based on available technical artifacts, map_unpack_to_slice refers to a function or operation associated with the internal testing and integrity verification of map objects in the Soroban host [2]. Specifically, references to map_unpack_to_linear_memory have appeared in the repository's test observations, indicating that this category of functions is used to facilitate the conversion or unpacking of map structures into linear memory or slice-based representations for internal processing [2]. These operations are part of the broader system used for ensuring map integrity, rejecting invalid configurations (such as out-of-order, duplicate, or oversized maps), and managing data conversions between the guest (smart contract) and host environments [3][4]. It is not a standard Rust library function or a general-purpose programming term, but rather an implementation-specific detail of the Soroban environment's host-side logic [1].
Citations:
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 6984
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 1480
🏁 Script executed:
Repository: Bonizozo/stellar-tipjar-contracts
Length of output: 15593
Decode
DataKey::Pausewith a v1 type.Soroban SDK 26.1.0 requires the stored map length to match
PauseState’s four fields. A v1 pause map has three fields, soget::<_, PauseState>fails before migration writesDataVersion. Read the value as a private three-fieldPauseV1, then constructPauseStatewithnotes: None.🤖 Prompt for AI Agents