🚨 EVERY CI CHECK MUST PASS. NO EXCEPTIONS. 🚨
Your PR will not be reviewed or merged unless every single job on the CI workflow is green: Formatting, Clippy, Build Optimized WASM, Tests, Dependency Audit, Coverage, and SDK Error Code Parity. No skipped jobs, no "unrelated failure", no #[ignore] to make a test go away. If a check is red, the work is not finished.
Summary
contracts/marketx/src/ contains 1,013 lines of Rust that no compiler, linter, formatter, or test runner has ever looked at. Four files are physically present in the crate directory but are not declared as modules anywhere, so as far as rustc is concerned they do not exist.
The worst of it: 13 tests that were written to protect fee arithmetic against overflow, and to protect the volume/fee-tier logic, have never once executed — and they no longer even compile.
contracts/marketx/src/test_integer_safety.rs 205 lines, 7 #[test] NEVER RUN
contracts/marketx/src/test_volume.rs 194 lines, 6 #[test] NEVER RUN
contracts/marketx/src/tarpaulin.rs 407 lines NEVER COMPILED
contracts/marketx/src/Automation.rs 0 lines EMPTY
lib.rs declares exactly three modules — errors, types, and test:
$ grep -rn "^\s*\(pub \)\?mod " contracts/marketx/src/*.rs
lib.rs:97:mod errors;
lib.rs:98:mod types;
lib.rs:130:mod test;
test_integer_safety.rs:14:mod integer_safety_tests {
test_volume.rs:14:mod volume_tests {
The last two lines are the trap. Both files open an inner module (mod integer_safety_tests {), which reads like the file is wired up. It is not — nothing ever declares mod test_integer_safety;. There are no #[path] attributes anywhere in the crate either.
Proof that the toolchain cannot see these files
I appended deliberately invalid Rust to tarpaulin.rs:
this is not valid rust at all !!! {{{Then ran the three jobs that are supposed to catch exactly this:
$ cargo check --all-targets
Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.98s
$ cargo clippy --all-targets -- -D warnings
Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.31s
$ cargo fmt --all -- --check
(no output — passed)
All three passed with syntactically invalid Rust sitting in the crate. Reverted afterwards; main is unmodified.
Part 1 — 13 safety tests that never ran, and no longer compile
These are not throwaway tests. Look at what they were written to defend:
test_integer_safety.rs — fee arithmetic edge cases:
test_zero_amount_returns_zero_feetest_small_amount_rounds_downtest_exact_divisiontest_remainder_handled_correctlytest_large_amount_no_overflowtest_multiple_escrows_accumulate_safelytest_zero_fee_bps_returns_zero
test_volume.rs — volume tracking and fee tiers:
test_volume_updated_after_escrow_releasetest_tier_calculation_from_volumetest_whitelist_prevents_feetest_default_tiers_set_on_initializetest_volume_accumulatestest_high_volume_tier_3
I wired both files in temporarily to see what would happen:
$ cargo test --lib
error[E0432]: unresolved import `crate::Client`
--> contracts/marketx/src/test_integer_safety.rs:16:17
error[E0432]: unresolved import `crate::Client`
--> contracts/marketx/src/test_volume.rs:20:17
error[E0599]: no function or associated item named `random` found for struct `soroban_sdk::Address`
error[E0061]: this method takes 4 arguments but 3 arguments were supplied
error[E0106]: missing lifetime specifier
error[E0308]: mismatched types
...
15 compile errors. The breakage dates the rot precisely:
Address::random does not exist any more. It was removed from soroban-sdk testutils and replaced by Address::generate in SDK 20. This crate is on soroban-sdk 25.1.1. These tests were written against a pre-20 SDK and have been dead through every SDK bump since.crate::Client no longer exists — the generated client type was renamed.initialize takes 4 arguments, the tests pass 3 — the constructor signature changed underneath them.
So the crate reports 128 tests passing and looks healthy, while the tests specifically covering overflow safety in fee math have been silently absent for roughly two years of SDK upgrades.
Part 2 — tarpaulin.rs is a stale, divergent second copy of ContractError
tarpaulin.rs is not a coverage config file, despite the name. It is a 407-line duplicate of the ContractError enum, complete with rich doc comments the real enum does not have:
// contracts/marketx/src/tarpaulin.rs
#[contracterror]
pub enum ContractError {
/// Caller is not the contract admin.
///
/// This error is returned when a function requires admin privileges
/// but the caller is not the configured admin address.
///
/// **Used in:** `assert_admin()`, `set_fee_percentage()`
NotAdmin = 1,The live enum is errors.rs. They have diverged badly:
|
variants |
errors.rs (real, compiled) |
58 |
tarpaulin.rs (dead) |
17 |
The dead copy is missing 41 variants — every one of the Dispute Resolution V2 codes (130–153), all the migration codes (170–174), the oracle codes (175–177), mediation (166–168), timelock (110–111, 180–181), and the entire group-buy range (120–123).
Two things make this actively harmful rather than merely untidy:
- The documentation is on the wrong copy.
tarpaulin.rs carries the **Used in:** cross-references and multi-paragraph explanations for each error. errors.rs — the one that actually compiles into the contract and drives the SDK — mostly has one-line comments or none. A contributor who searches the tree for NotAdmin and finds the well-documented version is reading a file that is not real.
- It defeats
#[contracterror] collision safety. Two #[contracterror] enums with the same name and overlapping discriminants is precisely the kind of thing that silently produces wrong error codes if the dead file is ever wired in by someone trying to "fix the unused file warning".
Good news, and worth stating so nobody re-investigates it: scripts/check_error_parity.py reads the correct file. It hardcodes contracts/marketx/src/errors.rs, so the SDK Error Code Parity job is genuinely validating the live enum. That check is fine. The problem is purely the phantom second copy.
Part 3 — Automation.rs, and why nothing flagged any of this
contracts/marketx/src/Automation.rs is 0 bytes, and its capitalised filename does not follow Rust module naming. It is a leftover.
The check that should have caught all of this is the Coverage job — and it cannot fail:
- name: Run coverage
run: cargo tarpaulin --all-features --workspace --timeout 120 --out Stdout
There is no --fail-under. Tarpaulin computes a coverage percentage, prints it to stdout, exits 0, and nobody reads it. The job is green whether coverage is 90% or 9%. A coverage job with no threshold is decoration: it costs CI minutes and a cargo install cargo-tarpaulin on every run, and it enforces nothing.
That is the through-line for this whole issue. Dead files survive because no job can see them, and the one job whose entire purpose is to notice missing coverage is configured so that it can never complain.
Scope of work
1. Revive the 13 dead tests.
- Declare both modules in
lib.rs under #[cfg(test)].
- Repair the 15 compile errors:
Address::random → Address::generate, fix the client type import, correct the initialize arity, add the missing lifetimes.
- Make them pass. If a revived test fails against current behaviour, that is a finding, not a nuisance — say so in the PR and fix the contract or justify the changed expectation. Do not delete a test to make the suite green, and do not
#[ignore] it.
- Expected result: 128 → 141 tests.
2. Resolve tarpaulin.rs. Delete it, and migrate its genuinely useful doc comments onto the surviving variants in errors.rs first. Every error the live enum defines should be documented at least as well as the dead copy documented its 17. Do not simply wire the dead enum in — it is missing 41 variants.
3. Delete Automation.rs.
4. Give the Coverage job teeth. Add --fail-under at or slightly below the coverage figure the job reports once the revived tests are in, so the number can only go up. Record the baseline percentage in the PR description.
5. Stop this from recurring. A file that is not in the module tree must not be able to sit in src/ unnoticed. Add a CI step that fails when a .rs file under contracts/*/src/ is not reachable from the crate root. A short script comparing the file list against declared modules is enough; put it in scripts/ next to check_error_parity.py and wire it into the workflow as its own job.
Acceptance criteria
Reproducing what I found
# The four orphaned files
ls -l contracts/marketx/src/{test_integer_safety,test_volume,tarpaulin,Automation}.rs
# Nothing declares them
grep -rn "^\s*\(pub \)\?mod " contracts/marketx/src/*.rs
# The toolchain cannot see them
printf '\nthis is not valid rust !!! {{{\n' >> contracts/marketx/src/tarpaulin.rs
cargo check --all-targets && cargo clippy --all-targets -- -D warnings && cargo fmt --all -- --check
git checkout -- contracts/marketx/src/tarpaulin.rs
# The enum has drifted: 58 live variants vs 17 dead ones
grep -cE "^\s+[A-Za-z_][A-Za-z0-9_]* = [0-9]+" contracts/marketx/src/errors.rs
grep -cE "^\s+[A-Za-z_][A-Za-z0-9_]* = [0-9]+" contracts/marketx/src/tarpaulin.rsPlease leave a comment before starting so the work is not duplicated.
🚨 EVERY CI CHECK MUST PASS. NO EXCEPTIONS. 🚨
Your PR will not be reviewed or merged unless every single job on the CI workflow is green:
Formatting,Clippy,Build Optimized WASM,Tests,Dependency Audit,Coverage, andSDK Error Code Parity. No skipped jobs, no "unrelated failure", no#[ignore]to make a test go away. If a check is red, the work is not finished.Summary
contracts/marketx/src/contains 1,013 lines of Rust that no compiler, linter, formatter, or test runner has ever looked at. Four files are physically present in the crate directory but are not declared as modules anywhere, so as far asrustcis concerned they do not exist.The worst of it: 13 tests that were written to protect fee arithmetic against overflow, and to protect the volume/fee-tier logic, have never once executed — and they no longer even compile.
lib.rsdeclares exactly three modules —errors,types, andtest:The last two lines are the trap. Both files open an inner module (
mod integer_safety_tests {), which reads like the file is wired up. It is not — nothing ever declaresmod test_integer_safety;. There are no#[path]attributes anywhere in the crate either.Proof that the toolchain cannot see these files
I appended deliberately invalid Rust to
tarpaulin.rs:Then ran the three jobs that are supposed to catch exactly this:
All three passed with syntactically invalid Rust sitting in the crate. Reverted afterwards;
mainis unmodified.Part 1 — 13 safety tests that never ran, and no longer compile
These are not throwaway tests. Look at what they were written to defend:
test_integer_safety.rs— fee arithmetic edge cases:test_zero_amount_returns_zero_feetest_small_amount_rounds_downtest_exact_divisiontest_remainder_handled_correctlytest_large_amount_no_overflowtest_multiple_escrows_accumulate_safelytest_zero_fee_bps_returns_zerotest_volume.rs— volume tracking and fee tiers:test_volume_updated_after_escrow_releasetest_tier_calculation_from_volumetest_whitelist_prevents_feetest_default_tiers_set_on_initializetest_volume_accumulatestest_high_volume_tier_3I wired both files in temporarily to see what would happen:
15 compile errors. The breakage dates the rot precisely:
Address::randomdoes not exist any more. It was removed fromsoroban-sdktestutils and replaced byAddress::generatein SDK 20. This crate is on soroban-sdk 25.1.1. These tests were written against a pre-20 SDK and have been dead through every SDK bump since.crate::Clientno longer exists — the generated client type was renamed.initializetakes 4 arguments, the tests pass 3 — the constructor signature changed underneath them.So the crate reports 128 tests passing and looks healthy, while the tests specifically covering overflow safety in fee math have been silently absent for roughly two years of SDK upgrades.
Part 2 —
tarpaulin.rsis a stale, divergent second copy ofContractErrortarpaulin.rsis not a coverage config file, despite the name. It is a 407-line duplicate of theContractErrorenum, complete with rich doc comments the real enum does not have:The live enum is
errors.rs. They have diverged badly:errors.rs(real, compiled)tarpaulin.rs(dead)The dead copy is missing 41 variants — every one of the Dispute Resolution V2 codes (130–153), all the migration codes (170–174), the oracle codes (175–177), mediation (166–168), timelock (110–111, 180–181), and the entire group-buy range (120–123).
Two things make this actively harmful rather than merely untidy:
tarpaulin.rscarries the**Used in:**cross-references and multi-paragraph explanations for each error.errors.rs— the one that actually compiles into the contract and drives the SDK — mostly has one-line comments or none. A contributor who searches the tree forNotAdminand finds the well-documented version is reading a file that is not real.#[contracterror]collision safety. Two#[contracterror]enums with the same name and overlapping discriminants is precisely the kind of thing that silently produces wrong error codes if the dead file is ever wired in by someone trying to "fix the unused file warning".Good news, and worth stating so nobody re-investigates it:
scripts/check_error_parity.pyreads the correct file. It hardcodescontracts/marketx/src/errors.rs, so theSDK Error Code Parityjob is genuinely validating the live enum. That check is fine. The problem is purely the phantom second copy.Part 3 —
Automation.rs, and why nothing flagged any of thiscontracts/marketx/src/Automation.rsis 0 bytes, and its capitalised filename does not follow Rust module naming. It is a leftover.The check that should have caught all of this is the
Coveragejob — and it cannot fail:There is no
--fail-under. Tarpaulin computes a coverage percentage, prints it to stdout, exits 0, and nobody reads it. The job is green whether coverage is 90% or 9%. A coverage job with no threshold is decoration: it costs CI minutes and acargo install cargo-tarpaulinon every run, and it enforces nothing.That is the through-line for this whole issue. Dead files survive because no job can see them, and the one job whose entire purpose is to notice missing coverage is configured so that it can never complain.
Scope of work
1. Revive the 13 dead tests.
lib.rsunder#[cfg(test)].Address::random→Address::generate, fix the client type import, correct theinitializearity, add the missing lifetimes.#[ignore]it.2. Resolve
tarpaulin.rs. Delete it, and migrate its genuinely useful doc comments onto the surviving variants inerrors.rsfirst. Every error the live enum defines should be documented at least as well as the dead copy documented its 17. Do not simply wire the dead enum in — it is missing 41 variants.3. Delete
Automation.rs.4. Give the
Coveragejob teeth. Add--fail-underat or slightly below the coverage figure the job reports once the revived tests are in, so the number can only go up. Record the baseline percentage in the PR description.5. Stop this from recurring. A file that is not in the module tree must not be able to sit in
src/unnoticed. Add a CI step that fails when a.rsfile undercontracts/*/src/is not reachable from the crate root. A short script comparing the file list against declared modules is enough; put it inscripts/next tocheck_error_parity.pyand wire it into the workflow as its own job.Acceptance criteria
test_integer_safety.rsandtest_volume.rsare declared, compile, and all 13 tests passcargo testreports 141 tests passing, 0 failing, 0 ignoredtarpaulin.rsis gone, and its documentation has been merged intoerrors.rsrather than discardedAutomation.rsis goneCoveragejob runs with--fail-under, and the chosen baseline is stated in the PR.rsfile under a crate'ssrc/is unreachable from the crate root — and the PR demonstrates it working by showing the job failing on a deliberately orphaned file, then passing once removedgrep -rn "^\s*\(pub \)\?mod " contracts/marketx/src/*.rsaccounts for every.rsfile in that directoryReproducing what I found
Please leave a comment before starting so the work is not duplicated.