Flaky Rust Tests

[jalextowle]

We're seeing an issue with calculate_absolute_max_short_execute where it is failing with a contract revert.

[dpaiton]

Just got two more of them on main branch:

failures:

---- lp::add::tests::fuzz_test_calculate_add_liquidity stdout ----
Error: Contract call reverted with data: 0x4e487b710000000000000000000000000000000000000000000000000000000000000011

Location:
    crates/hyperdrive-math/src/test_utils/agent.rs:87:25

---- short::open::tests::test_calculate_implied_rate stdout ----
Error: Contract call reverted with data: 0x4e487b710000000000000000000000000000000000000000000000000000000000000011

Location:
    crates/hyperdrive-math/src/test_utils/agent.rs:87:25

[dpaiton]

another unrelated failure on main

---- short::max::tests::fuzz_calculate_max_short stdout ----
thread 'short::max::tests::fuzz_calculate_max_short' panicked at crates/hyperdrive-math/src/short/max.rs:742:17:
expected (base=72369083.950666811963261164) < (budget=72942398.328880144557334009) * 0.001000000000000000 = 72942.398328880144557334

[dpaiton]

Regarding the contract call reverted error:

The contract error is a FixedPoint underflow due to subtraction happening in the MockER4626.sol::_getAccruedInterest() function:

function _getAccruedInterest() internal view returns (uint256) {
        if (_rate == 0) {
            return 0;
        }

        // base_balance = base_balance * (1 + r * t)
        uint256 timeElapsed = (block.timestamp - _lastUpdated).divDown(
            365 days
        );
        uint256 accrued = asset.balanceOf(address(this)).mulDown(
            _rate.mulDown(timeElapsed)
        );
        return accrued;
    }

The block.timestamp - _lastUpdated logic is failing because _lastUpdated is one block ahead of block.timestamp.

The bug is hard to reproduce because tests can pass 100s of times and then fail. Somehow adding println or console.log statements here and there makes it less likely to happen. So tests that run faster might increase the likelihood of failure. However, it can also happen in the normal build mode (not --release) & without print statements. It's also worth noting that the same command in our Python tests is not causing this problem. We do the same thing where we use RPC evm calls to snapshot and revert in loops in the tests.

This is happening intermittently so it's hard to compile a list of all tests that could possibly fail. I think the test would have to havechain.snapshot() and chain.revert() (which is almost all tests). This happens here in the rust repo. However, the test that is most consistently failing was test_calculate_implied_rate, followed by fuzz_calculate_absolute_max_short_execute. Both of these include time advancement & adjusting the variable rate. I think this might be the key: any test that does this has trouble reverting. If so, we can reduce the number of potentially flaky tests from all of them to about 11.

Attempted solutions

I tried advancing time after the revert, minting blocks, making RPCs to get block info, and including a sleep when the revert happens. None of these consistently fix the problem.

I also tried checking the block number after reverts:

            let vault = MockERC4626::new(
                bob.get_config().vault_shares_token,
                chain.client(BOB.clone()).await?,
            );
            vault.set_rate(variable_rate.into()).send().await?;

Next steps

I'm not sure how to fix this problem yet. But one thing we can do is make an isolated rust test that has nothing to do with hyperdrive, which we can then post as an issue on the foundry github.

[wakamex]

Why don't we just add a check for this edge-case which represents a broken state of the world into MockERC4626? We have full control over that function, and it's there just to facilitate our testing.

By broken state, I assume this edge case will never occur in production. I'm assuming it's entirely an artifact of something going wrong with our local node.

[dpaiton]

https://github.com/delvtech/hyperdrive/pull/1040 fixes this, following @wakamex 's suggestion.

I moved the max short issue to https://github.com/delvtech/hyperdrive-rs/issues/121

This issue is now resolved.