[PR #38] p3: test_rescue_transfersNativeBnb uses EOA recipient — masks known #117 multisig revert risk

[obchain]

Summary

test_rescue_transfersNativeBnb sends BNB to makeAddr("recipient"), which is an EOA (no fallback). The test passes because payable(to).transfer(2300 gas) succeeds against an EOA.

Issue #117 documents that rescue() uses Solidity's transfer() which forwards only 2300 gas. If the owner's cold wallet is a Safe multisig or any contract with a fallback that requires more than 2300 gas (e.g., Safe's fallback manager), the call will revert, permanently trapping native BNB in the contract.

The current test implicitly validates the happy path while actively masking the failure path. A test reviewer seeing green on test_rescue_transfersNativeBnb would not be alerted to the multisig revert risk.

Location

contracts/test/CharonLiquidator.t.sol — test_rescue_transfersNativeBnb

Fix

Add a test using a stub contract that consumes more than 2300 gas in its receive function:

contract GasHungryReceiver {
    uint256 public counter;
    receive() external payable {
        for (uint256 i; i < 100; ++i) { counter++; } // >2300 gas
    }
}

function test_rescue_transferNativeBnb_revertsOnContractRecipient() public {
    GasHungryReceiver receiver = new GasHungryReceiver();
    vm.deal(address(liquidator), 1 ether);
    vm.expectRevert();
    liquidator.rescue(address(0), address(receiver), 1 ether);
}

This test should be marked as documenting a known limitation (not blocking) while #117 is open, but it must exist to prevent regressions.

Refs #38, Refs #117

[obchain]

Fixed via commit 6447016. Added test_rescue_revertsWhenNativeRecipientRejects using a new in-file GasHungryReceiver contract whose receive() reverts unconditionally — guaranteed to fit inside the 2300-gas budget payable(to).transfer forwards, so the receiver's revert propagates and the outer rescue() call reverts.

The test pins down current behaviour:

• No BNB moves (rejector balance stays 0, liquidator balance untouched).
• Rescue to a contract that consumes >2300 gas in its receive fallback (Gnosis Safe, any proxy/guard hook contract) will DoS the rescue.

This documents — but does not remediate — the limitation raised by issue #117. The production fix (switch to (bool ok,) = to.call{value: amount}("") with a require) is tracked separately and will land on the ancestor branch; this test stays as a regression guard for the current transfer semantics until then.