[BUG] Multi-sig proposal approval permanently deadlocks when a higher-index signer approves before a lower-index signer

[N-thnI]

Priority: High

Description

governance::approve() enforces that approvals must arrive in strictly increasing signer-index order relative to the most recent approval, not relative to the set of signers who haven't yet approved. There's no way to remove/reset an approval and no proposal-cancellation path, so if any signer approves "out of turn," every signer with a lower configured index becomes permanently unable to approve — even if quorum could otherwise be met. In the worst case, the highest-index signer can unilaterally and permanently deadlock any proposal (threshold >= 2) just by approving first.

Location

engine-core/src/governance.rs:210-220 (approve)

Current Behavior

if !proposal.approved_by.is_empty() {
    let last_signer = proposal.approved_by.get(proposal.approved_by.len() - 1).unwrap();
    let last_index = signer_index(env, &last_signer);
    let current_index = signer_index(env, signer);
    if current_index <= last_index {
        panic_with_error!(env, GovError::InvalidSignerOrder);
    }
}

Expected Behavior

Any signer who hasn't yet approved should be able to approve at any time, in any order, until threshold is reached. The approved_by.contains(signer) check already prevents double-approval; ordering shouldn't gate the state transition.

Repro / Evidence

let (alice, bob, carol) = (signers[0], signers[1], signers[2]); // indices 0,1,2
init(&env, vec![&env, alice.clone(), bob.clone(), carol.clone()], 2); // 2-of-3
propose(&env, proposal(&env, 1, alice.clone()));

approve(&env, &carol, 1);   // index 2 -- allowed
approve(&env, &bob, 1);     // index 1 <= 2 -> panics InvalidSignerOrder
approve(&env, &alice, 1);   // index 0 <= 2 -> panics InvalidSignerOrder
// proposal id=1 can NEVER reach threshold=2 again -- stuck Pending forever

Impact

A single signer (malicious, confused, or just first-to-sign) can permanently block any governance proposal — including contract upgrades and protocol-parameter changes — regardless of whether remaining signers could otherwise satisfy quorum. No cancel/reset entrypoint exists.

Suggested Fix

Only reject if signer has already approved (approved_by.contains(signer)); drop the ordering requirement, or move any determinism need to how the audit event is published rather than gating the state transition.

Acceptance Criteria

• AC-1: Any not-yet-approved signer can approve regardless of prior approver's index.
• AC-2: New test out_of_order_approval_still_reaches_threshold reproducing the scenario — fails on current code, passes after fix.

Definition of Done

• Fix merged with all AC items checked
• Regression test passes in CI
• No new clippy warnings

[N-thnI]

See the note on #17 — these two issues currently ask for opposite fixes to the same signer-ordering enforcement in governance.rs::approve. Flagging both for maintainer triage before either is assigned.