Fix failing CI: botched merge-conflict damage and clippy violations - #240
Merged
chonilius merged 4 commits intoSep 7, 2026
Merged
Conversation
Merge commit 2fdf95d ("Merge branch 'main' into feature/upgrade-pause-pagination-separation") left conflict-marker remnants in contracts/maintenance-pool/src/error.rs and test.rs — the <<<<<<</=======/>>>>>>> lines themselves were removed, but the branch-name text on the marker lines (" feature/upgrade-pause-pagination- separation", " main") was left behind mid-file, and in a few spots got concatenated onto adjacent code. This has been failing `cargo build --target wasm32v1-none --release` (a hard compile error — literal "feature/upgrade-pause-pagination-separation" as enum-body content) on every push to main since 2026-08-31. - error.rs: removed the two stray branch-name lines, and resolved the actual conflict underneath them — both sides of the merge had added a new Error variant at discriminant 13 (ContractPaused from the pause feature, DepositCountOverflow from main), so the "resolution" that dropped the conflict markers kept both variants but left them colliding on the same value. Kept ContractPaused = 13, moved DepositCountOverflow to 14. - test.rs: the same botched resolution merged two entirely different tests — test_unpause_restores_deposit (added by the pause feature) and test_deposit_rejects_when_deposit_count_would_overflow (already on main) — into one malformed function, dropping the first test's closing brace and the second test's own setup code. Recovered both tests' original, complete bodies from the merge's two parent commits (git show 40838e3 / 9e8d53f) and restored them as separate functions.
Same root cause as the previous commit (merge 2fdf95d), same file pattern, in contracts/milestones/src/test.rs: - A stray " feature/upgrade-pause-pagination-separation" line sat directly above test_pause_blocks_commitment_paths_but_allows_cancel — removed, body was otherwise intact. - test_unpause_restores_milestone_creation lost its entire body: the function's opening brace was immediately followed by the (unrelated) MockPanicToken mock's struct/impl block, meaning the mock ended up nested inside the test instead of being its own top-level item, and the test itself asserted nothing. Recovered the real body from the merge's feature-branch parent (git show 40838e3) and restored MockPanicToken to its correct top-level position right after — where it already needs to be, since test_release_issue_all_or_nothing_ revert_with_blocked_recipient right below it depends on it. - test_state_machine_allocate_rejects_duplicate_allocation was similarly fused with test_unpause_restores_milestone_creation's body and missing its own setup entirely. Recovered its real, self-contained body from the merge's main-branch parent (git show 9e8d53f). Also fixes the 2 clippy violations this file had once `cargo test` compiled far enough to be linted at all: a collapsible `if let Ok .. { if !contains { .. } }` in the fuzz-invariant harness, and 5 occurrences of `.len() > 0` where `!.is_empty()` is clearer (clippy::len_zero, denied by this repo's `-D warnings` clippy invocation).
Once the merge-conflict damage stopped `cargo test`/`cargo clippy` from failing to even compile, 4 more pre-existing clippy warnings surfaced (this repo's CI runs clippy with -D warnings): - common/src/lib.rs: validate_fee_change (issue MergeFi#20's fee-change-limit guard) returned Result<(), ()> — clippy::result_unit_err flags bare unit-error results since callers get no information from a match/`?`. Added a small FeeChangeError enum (mirroring split.rs's SplitError, the same "deliberately small and generic, map to your own Error enum" pattern already established in this crate) and used `new_fee.abs_diff(old_fee)` instead of the equivalent manual if/else subtraction (clippy::manual_abs_diff). - escrow/src/lib.rs, milestones/src/lib.rs: dropped a needless `&` on `env.current_contract_address()` in 4 fund()/contribute() token transfer calls (clippy::needless_borrows_for_generic_args) — the token client's generic transfer signature already accepts the owned value.
`cargo fmt --check` (run by CI's test job, failing on main since before these fixes landed) had a backlog of pre-existing formatting drift in files this PR doesn't otherwise touch — trailing whitespace on blank lines and a few over-long expressions rustfmt wraps differently. Ran `cargo fmt` across the workspace to clear it; no behavior changes, verified via the full test suite before and after.
|
@gideononiru is attempting to deploy a commit to the chonilius' projects Team on Vercel. A member of the Team first needs to authorize it. |
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
2 participants
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
CI on
mainhas been failing since 2026-08-31 — both jobs,wasm-buildandtest.wasm-buildfailed with a hard compile error: literal branch-name text (feature/upgrade-pause-pagination-separation) sitting in the middle of an enum body incontracts/maintenance-pool/src/error.rs. Traced this to merge commit2fdf95d("Merge branch 'main' into feature/upgrade-pause-pagination-separation"): the<<<<<<</=======/>>>>>>>conflict-marker lines were removed, but the branch-name text on the marker lines was left behind mid-file instead, in some spots concatenated directly onto real code. The same botched resolution touchedcontracts/maintenance-pool/src/test.rsandcontracts/milestones/src/test.rs, in each case fusing two unrelated test functions into one malformed function and dropping the other's body/setup entirely.testfailed atcargo fmt --check(never got to clippy/tests). Once the merge damage was fixed and the workspace could actually compile, clippy (run with-D warnings) surfaced 6 more pre-existing violations that had never been checked before.Merge-conflict recovery
Used
git show <parent>:<path>against2fdf95d's two parent commits to recover each side's real, complete function body, rather than guessing at the lost content:error.rs: both merge sides had independently added a newErrorvariant at the same discriminant (13) —ContractPaused(the pause feature) andDepositCountOverflow(already on main). Kept both, moved the second to 14.maintenance-pool/test.rs: recoveredtest_unpause_restores_depositandtest_deposit_rejects_when_deposit_count_would_overflowas two separate, complete tests (they'd been merged into one Frankenstein function with the first's closing brace and the second's setup code both dropped).milestones/test.rs: recoveredtest_unpause_restores_milestone_creation's entire body (it had none — the function's{was immediately followed by the unrelatedMockPanicTokenmock, which is now restored to its correct top-level position), andtest_state_machine_allocate_rejects_duplicate_allocation's own self-contained body.Clippy fixes
validate_fee_change(issuefee_bpsis immutable afterinitialize— design and implement a secure, bounded update mechanism #20's fee-change-limit guard, inmergefi-common) returnedResult<(), ()>— added a smallFeeChangeErrorenum, mirroring the existingSplitErrorpattern in the same crate, and replaced a manual abs-diff if/else with.abs_diff().&onenv.current_contract_address()in escrow/milestones token-transfer calls.ifand 5.len() > 0→!.is_empty()in the milestones fuzz-invariant test harness.Also ran
cargo fmtto clear a backlog of pre-existing formatting drift (trailing whitespace, a few rewrapped expressions) in files this PR doesn't otherwise touch.Verified locally end-to-end:
cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(138 tests),cargo doc --workspace --no-deps --document-private-itemswithRUSTDOCFLAGS=-D warnings, andcargo build --target wasm32v1-none --releaseall pass clean. (Did not independently verify thecargo llvm-covcoverage-report step — the tool isn't installable in this sandbox without network access to crates.io — but it wraps the same test run that already passes, and the workflow doesn't gate on a coverage threshold.)Test plan
cargo fmt --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— 138/138 passingRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --document-private-items— cleancargo build --target wasm32v1-none --release— succeedscargo llvm-cov --workspace— not independently run locally (tool unavailable offline); underlying tests pass