fix(platform-wallet): build data contract config at the protocol-required version#3881
Conversation
…ired version The wallet contract-create path hardcoded `"$formatVersion": "0"` when tagging the assembled `DataContractConfig`. Since protocol v12 the network's data-contract-create basic-structure validator enforces `config().version() >= contract_versions.config.min_version`, and that minimum is 1 for v12 (V0 config lacks `sized_integer_types`). Every contract the wallet built therefore carried a V0 config and was rejected with "config version 0 is not supported, minimum version is 1", blocking all contract creation (document and token contracts) from clients such as the SwiftExampleApp. Now the config is tagged with the running `PlatformVersion`'s `contract_versions.config.default_current_version` (which equals `min_version` on v12), and a config block is always inserted — including when the caller supplies no config — so the version tag is explicit rather than relying on the serialization format's serde default. A flags-only or empty config dict deserializes into a valid V1 because every `DataContractConfigV1` field carries a serde default (`sized_integer_types: true` included), so no extra field population is required. The token-schema `"$formatVersion": "0"` emitted by QuickBasicTokenView is left unchanged: `TokenConfiguration` only has a V0 variant and there is no token-config min-version gate on v12, so V0 is still the only valid token-configuration version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates contract creation to use protocol-defined config format versions instead of hardcoded defaults. The change derives the version from the running ChangesData Contract Config Format Version
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit 4c31f24) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Small, focused fix that correctly tags the assembled DataContractConfig with the running PlatformVersion's contract_versions.config.default_current_version, resolving the v12 testnet rejection where the basic-structure validator enforces config().version() >= contract_versions.config.min_version. Implementation is sound; no blocking issues. Two suggestions: add a regression test pinning the version-tag selection, and decide explicitly how to handle a caller-supplied $formatVersion (currently silently overridden).
🟡 1 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-platform-wallet/src/wallet/identity/network/contract.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/contract.rs:258-271: Add a regression test pinning the config $formatVersion to the protocol's required version
This PR fixes a real consensus-rejection bug (v12 rejects V0 DataContractConfig because basic-structure validation requires config().version() >= contract_versions.config.min_version). The whole correctness boundary of the change lives in this JSON assembly block, but there's no test in rs-platform-wallet that exercises it, so a future refactor could silently reintroduce the hardcoded "0" or omit the config block without failing CI.
Extracting the config-Map assembly into a small pure helper, e.g. `fn build_config_object(config_json: Option<&str>, version: u16) -> Result<serde_json::Map<String, serde_json::Value>, PlatformWalletError>`, makes this testable without the SDK runtime / signer / wallet_manager dependencies. Three cases are sufficient to lock the fix down:
- `None` → resulting object has `$formatVersion == default_current_version.to_string()` for the current PlatformVersion (1 on v12).
- `Some(flags-only object)` → caller fields preserved, version tag set to the protocol default.
- `Some(non-object)` (array/string/number) → returns `PlatformWalletError::InvalidIdentityData`.
Ideally a fourth case round-trips through `DataContractInSerializationFormat` + `DataContract::try_from_platform_versioned` at the latest PlatformVersion to assert the assembled bytes actually validate. The PR description's own test checklist is unchecked — given this is a fix for a v12 consensus rejection that blocked all contract creation, a guard test here is well worth the few lines.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3881 +/- ##
=============================================
- Coverage 87.22% 71.20% -16.03%
=============================================
Files 2641 20 -2621
Lines 328569 2837 -325732
=============================================
- Hits 286597 2020 -284577
+ Misses 41972 817 -41155
🚀 New features to boost your workflow:
|
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "bd4a9a9822c33e2d6d494f8bdd30db3d87cd27471a461e62a7ce5b58254f43ff"
)Xcode manual integration:
|
|
QA follow-up (testnet, found while exercising this fix end-to-end): this change is necessary but not sufficient on its own. It reads the config version from I tried refining both the create and update paths to build the config at the client's compiled Conclusion: the real fix is making the wallet's |
…h a regression test
Extract the config-Map assembly from `create_data_contract_with_signer`
into a pure, unit-testable `build_config_object(config_json, version)`
helper, and add a `#[cfg(test)] mod tests` covering:
- None / empty input -> `$formatVersion == default_current_version`
- flags-only object -> caller fields preserved + version tag set
- caller-supplied `$formatVersion` -> overwritten with the protocol
version (deliberate; the helper always emits a network-acceptable tag)
- non-object JSON (array/string/number) -> InvalidIdentityData
- round-trip: helper output validates as a DataContract at the latest
PlatformVersion (the assertion that would have caught the original
hardcoded-"0" v12 consensus rejection at unit-test time)
Behavior of the create path is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward prior findings: prior-1 and prior-2 are both FIXED in the latest delta. New findings in the latest delta: one nitpick — the public Swift docstring for createDataContract still claims the library injects $formatVersion: "0" if missing, which now contradicts the Rust helper's documented unconditional-overwrite-with-protocol-required-version behavior introduced by this PR. The Rust fix itself looks correct, well-scoped, dependency-direction clean, and the new tests (build_config_object_* plus the latest-version DataContract::try_from_platform_versioned round-trip) would have caught the original v12 rejection bug.
💬 1 nitpick(s)
1 additional finding(s) omitted (not in diff).
Issue being fixed or feature implemented
Creating ANY data contract through the SwiftExampleApp (and any other client routing through
platform-wallet) is rejected on protocol v12 (testnet) with:This blocks all contract creation — both token and document contracts.
Root cause:
packages/rs-platform-wallet/src/wallet/identity/network/contract.rs(create_data_contract_with_signer) hardcoded"$formatVersion": "0"when tagging the assembledDataContractConfig. Since protocol v12,DataContractConfigV0 is no longer accepted — the data-contract-create basic-structure validator (rs-drive-abci/.../data_contract_create/basic_structure/v1/mod.rs) enforcesconfig().version() >= platform_version.dpp.contract_versions.config.min_version, and that minimum is1for v12 (seers-platform-version/.../dpp_contract_versions/v3.rsandv4.rs:config.min_version = 1, "V0 config no longer accepted — lacks sized_integer_types"). So every contract the wallet built carried a V0 config and was rejected.What was done?
contract.rs, the assembled config is now tagged with the runningPlatformVersion'sdpp.contract_versions.config.default_current_version(which equalsmin_versionon v12) instead of a hardcoded"0".configblock is now always inserted — including when the caller passes noconfig_json— so the version tag is explicit rather than relying on the serialization format's#[serde(default = "DataContractConfigV1::default_with_version")].DataContractConfigV1is#[serde(rename_all = "camelCase", default)], so every field (includingsized_integer_types: true) has a serde default. No extra field population is needed.config_jsonnow returns a clearInvalidIdentityDataerror instead of being silently dropped.let platform_version = self.sdk.version();binding (hoisted earlier for the version lookup).Token schema left unchanged (investigated):
QuickBasicTokenView.swiftemits"$formatVersion": "0"for the token schema.TokenConfigurationonly has aV0variant and there is no token-config min-version gate on v12 (token_versionscarries onlyvalidate_structure_interval), so V0 is still the only valid token-configuration version. Bumping it would break deserialization, so it was intentionally left alone.How Has This Been Tested?
cargo check -p platform-walletpasses (compiles clean, including transitivedpp/drive/dash-sdk).cargo fmt --all --checkpasses.Breaking Changes
None. This is a client-side fix in the
platform-walletcrate; it changes the config version the wallet emits to match what the network already requires. No consensus or wire-format change.Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests