[BUG]: Escrow settlement permanently blocked by irreversible status transition after any successful refund call

[Odhiambo526]

Severity

High (breaks core features)

Description

In Escrow.sol, after escrow() sets statuses[escrowId] = EscrowStatus.CREATED, the function settle(bytes32[] calldata escrowIds) contains if (statuses[escrowId] != EscrowStatus.CREATED) revert InvalidStatus(); followed by statuses[escrowId] = EscrowStatus.FINALIZED; and the ISettler.read() check; conversely, refundDepositor(), refundRecipient(), and refund() first enforce if (block.timestamp <= _escrow.refundTimestamp) revert RefundInvalid();, then read the current status and execute if (status == EscrowStatus.CREATED) { statuses[escrowId] = EscrowStatus.REFUND_DEPOSIT; } else if (status == EscrowStatus.REFUND_RECIPIENT) { statuses[escrowId] = EscrowStatus.FINALIZED; } else revert InvalidStatus(); (or the symmetric path for refundRecipient), after which any subsequent call to settle() on the same escrowId unconditionally reverts with InvalidStatus() regardless of the return value of ISettler(_escrow.settler).read(_escrow.settlementId, _escrow.sender, _escrow.senderChainId) or the state of any LayerZero message.

Proof of Concept

add this to escrow.t.sol

    function testSettleRevertsInvalidStatusAfterSuccessfulRefund() public {
        bytes32[] memory ids = new bytes32[](1);

        IEscrow.Escrow memory eOk = _createEscrowData(1000, 800);
        eOk.salt = bytes12(uint96(200));
        ids[0] = _createAndFundEscrow(eOk);
        vm.prank(settlerOwner);
        settler.write(sender, eOk.settlementId, 1);
        vm.expectEmit(true, false, false, false);
        emit EscrowSettled(ids[0]);
        escrow.settle(ids);
        assertEq(uint256(escrow.statuses(ids[0])), uint256(IEscrow.EscrowStatus.FINALIZED));
        assertEq(token.balanceOf(recipient), 1000);
        assertTrue(settler.read(eOk.settlementId, eOk.sender, eOk.senderChainId));

        IEscrow.Escrow memory e0 = _createEscrowData(1000, 800);
        e0.salt = bytes12(uint96(201));
        ids[0] = _createAndFundEscrow(e0);
        vm.prank(settlerOwner);
        settler.write(sender, e0.settlementId, 1);
        vm.warp(block.timestamp + 2 hours);
        escrow.refundDepositor(ids);
        assertTrue(settler.read(e0.settlementId, e0.sender, e0.senderChainId));
        vm.expectCall(
            address(settler),
            abi.encodeCall(ISettler.read, (e0.settlementId, e0.sender, e0.senderChainId)),
            uint64(0)
        );
        vm.expectRevert(Escrow.InvalidStatus.selector);
        escrow.settle(ids);

        IEscrow.Escrow memory e1 = _createEscrowData(1000, 800);
        e1.salt = bytes12(uint96(202));
        ids[0] = _createAndFundEscrow(e1);
        vm.prank(settlerOwner);
        settler.write(sender, e1.settlementId, 1);
        vm.warp(block.timestamp + 2 hours);
        escrow.refundRecipient(ids);
        vm.expectRevert(Escrow.InvalidStatus.selector);
        escrow.settle(ids);

        IEscrow.Escrow memory e2 = _createEscrowData(1000, 800);
        e2.salt = bytes12(uint96(203));
        ids[0] = _createAndFundEscrow(e2);
        vm.prank(settlerOwner);
        settler.write(sender, e2.settlementId, 1);
        vm.warp(block.timestamp + 2 hours);
        escrow.refund(ids);
        vm.expectRevert(Escrow.InvalidStatus.selector);
        escrow.settle(ids);
    }

• First block (eOk): Shows the intended happy path which is, settle() succeeds when status == CREATED, correctly calls settler.read(), emits EscrowSettled, transfers the full escrowAmount, and ends in FINALIZED.

• Second block (e0): Shows the vulnerable path which is, after refundDepositor() succeeds, even though settler.read() returns true (explicitly asserted), settle() reverts with InvalidStatus() without ever calling settler.read() (proven by vm.expectCall(..., 0)).

• The vm.expectCall(..., uint64(0)) is an excellent technical assertion: it proves that the ISettler.read() line inside settle() is never executed once the status has left CREATED. This directly validates the core problem: the status transition is irreversible and short-circuits the settlement check.

forge test --match-test testSettleRevertsInvalidStatusAfterSuccessfulRefund -vvv
[⠊] Compiling...
No files changed, compilation skipped

Ran 1 test for test/Escrow.t.sol:EscrowTest
[PASS] testSettleRevertsInvalidStatusAfterSuccessfulRefund() (gas: 1193589)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 20.57ms (14.51ms CPU time)

Ran 1 test suite in 24.57ms (20.57ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Additional Context

Impact

Once any refund* function succeeds on an escrowId, settle() becomes unreachable for that escrowId for the remainder of the contract's lifetime.

Mitigation

Replace the strict CREATED check in settle() with if (statuses[escrowId] == EscrowStatus.FINALIZED) revert InvalidStatus(); and perform the status transition to FINALIZED only after a successful ISettler.read() check.