fix(contracts): restore compilation across all four contract crates - #1416
Draft
broda-spendy wants to merge 1 commit into
Draft
broda-spendy wants to merge 1 commit into
broda-spendy wants to merge 1 commit into
Conversation
main did not compile. \cargo check --all\ now passes and 488 of 517 tests pass. Merge damage: a botched merge of two divergent total_assets() bodies in lib.rs left a validate_price_data() call with a dangling delimiter; is_performance_fee_enabled was missing a closing brace that swallowed the following ~1200 lines; DataKeyExt::Risk was declared twice; bridge-compat used #[contracterror] without importing it (all 21 of its errors); Cargo.lock had no bridge-compat entry, so --locked failed outright. errors.rs declared 53 variants, but the Soroban error-enum spec caps an enum at 50 cases, so #[contracterror] panicked with LengthExceedsMax and took down every use of VaultError with it. Commit 8154517 pushed it from 50 to 53. The three performance-fee codes now reuse existing codes with matching semantics, following the pattern errors.rs already documents for the oracle and rescue flows. Codes 1-50 are unchanged, so the integrator contract in docs/api/ERROR_CODE_CATALOG.md is unaffected. Nine error variants referenced from commit 1412333 were never declared in any revision of errors.rs. Mapped each onto the closest existing code. Also fixed an over-length contract function name, three over-length symbol_short! topics, and tests reading instance storage outside a contract invocation.
|
@broda-spendy Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This was referenced Sep 25, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maindoes not compile. This restorescargo check --workspace --all-targets --lockedto green, and brings the test suite from "never compiled" to 488 of 517 passing.cargo fmt --all -- --check— clean (was failing with 14 diffs inbridge-compat).cargo check --workspace --all-targets --locked— clean.cargo test --all— 488 passed, 29 failed (was: 0 compiled).cargo build -p mock-strategy --target wasm32-unknown-unknown --release— clean.What was broken
Merge damage. A botched merge of two divergent
total_assets()bodies (a328ce8, "Merge branch 'main' into hardening") left avalidate_price_data(...)call with a dangling)and two conflicting oracle-price paths interleaved. I restored themain-side version, which is what the surrounding signature and helpers expect. Separately,is_performance_fee_enabledwas missing a closing}, which swallowed the following ~1200 lines into the function body — that one predatesa328ce8and is present in both parents.DataKeyExt::Risk(RiskExtKey)was declared twice.bridge-compatused#[contracterror]without importing it, which accounted for all 21 of its errors.Stale
Cargo.lock.8154517addedbridge-compatto the workspace but never regenerated the lock file, socargo --lockedfailed outright in CI. Regenerated.errors.rsexceeded the error-enum cap.ScSpecUdtErrorEnumV0allows at most 50 cases.errors.rsdeclared 53, so#[contracterror]panicked withLengthExceedsMax— and because that panic aborts the derive, everyuse crate::errors::VaultErrordownstream failed too, which is what produced the 478-site cascade. Commit8154517took it from 50 to 53.The file's own comments already document the constraint twice, and
RescueUnauthorized = 50notes that the rescue flow deliberately reuses existing codes to stay inside it. I applied the same pattern: the three performance-fee codes reuse existing codes with matching semantics, documented inline.Numeric codes 1–50 are unchanged, so the integrator contract in
docs/api/ERROR_CODE_CATALOG.mdis untouched. The catalog documents 1–49 and never mentioned 50–53. Nothing is deployed on any network (every contract ID indeployments/is empty) and there are no git tags, so there is no live ABI to break. I flagged the semantics I chose in the diff comments for review.Nine error variants that never existed. Commit
1412333introducedstrategy_validation.rs,governance_validation.rs,rounding_consistency.rs, andoperational_safety_tests.rs, all referencing nineVaultErrorvariants that appear in no revision oferrors.rs. I checked every revision — the referencing side is the broken one. Mapped each to the closest existing code by semantics (InvalidStrategyResponse→InvalidAmount,InsufficientGovernanceVotes→QuorumNotReached, and so on) following the same reuse convention.SDK 22 limits.
symbol_short!rejects topics over 9 characters and contract function names over 32. Renamedvault_pause/vault_unpause/pause_failtovpaused/vunpause/paufail(matching thepaused/unpausedtopicspause/unpausealready publish) andstrategy_switch_cooldown_remaining(34) tostrategy_switch_cooldown_left.ABI changes worth your review
Three, all consequences of making the code compile:
utilization_bpsandeffective_fee_bpsnow returnResult<i128, VaultError>. They calltotal_assets, which is already fallible — the previous signature was unsound.report_benji_yieldwas updated.strategy_switch_cooldown_remaining→strategy_switch_cooldown_left(name-length limit; no caller existed).total_assetsis unchanged in behaviour; only its callers gained?.Not fixed — please triage separately
29 tests fail on assertion values, not compilation. These are pre-existing behavioural drift, not something this branch introduces: I verified the failing tests are byte-identical to
upstream/main. Examples —test_vault_with_benji_strategyexpects 50 assets but gets 25;fee_math::test_fee_invariant_large_balancestripsfee overflowonamount.checked_mul(fee_bps). The 10oracle_failure_testsare a cluster worth one issue on their own.Also untouched:
contracts/vault/ROUNDING_POLICY.mdanddocs/DYNAMIC_FEES.mdreference event topics I renamed, andstrategy_switch_cooldown_remainingmay appear in frontend code — worth a grep before deploy.Notes
Cargo.lockis tracked in this repo, so regenerating it is part of the fix.x86_64-pc-windows-gnutoolchain. mingw cannot link a Windows cdylib (export ordinal too large), so test binaries were built withcrate-typetemporarily reduced torlib; the manifests are unchanged in this PR. CI builds on Linux.Address::from_string(&String::new(env))could never have compiled, and thebridge-compattests read instance storage outside a contract invocation, which SDK 22 rejects.Closes #1303