chore(tests): Add BasisModule <> SlippageIssuance Integration Tests

[0xSachinK]

Review suggestions

• Remove duplicated tests
  • Removed duplicated issuance tests in 7fa3e3e
  • Added comments to explain which parts are tested and which skipped along with reasons.
• Tests for redemption when funding is non-zero (Added here and here)
• Use getRedemptionAdjustment in a redemption flow to test the UI flow (Added in 9b88be4)

Additions to unit tests

• Setting the performance fee after withdrawing with different amounts (897a00b)
• Deposit entire withdrawn amount (b46101e)

[cgewecke]

(Closing and re-opening to see if that fixes semantic release bot failure)

[cgewecke]

I wonder if we should use this to look at scenarios that are more specific to the Basis module. Its moduleIssuance and component hooks are identical with PerpV2LeverageModuleV2 which already runs all these tests.

Might be interesting to remove cases duplicated by the other test file and look at:

• redemption where funding is non-zero
• what happens when calling methods like withdrawFundingAndAccrueFees
• using getRedemptionAdjustments in a redemption flow.
• reproducing the revert Iosiro reported when depositing after a withdrawal
• setting the performance fee after withdrawing with different amounts.

I know some of these are annoying to set very specific expectations for but it seems useful to exercise different scenarios and make sure there are no surprises when methods with new logic are called in combination.

@0xSachinK What do you think?

[0xSachinK]

@cgewecke

1. Added in f66a973d4.
2. Tests for withdrawFundingAndAccrueFees are added as part of unit tests.
3. How do we use getRedemptionAdjustments in the redemption flow? Can you give a specific example? We do have tests for getRedemptionAdjustments as part of unit tests.
4. Added tests for depositing entire withdrawn amount in b46101. I was unable to reproduce the bug reported by iosiro, but working on it.
5. Added tests for setting performance fee after pushing tracked settled funding to zero by withdrawing and funding in 897a00b.

[cgewecke]

How do we use getRedemptionAdjustments in the redemption flow? Can you give a specific example? We do have tests for getRedemptionAdjustments as part of unit tests

I'm thinking of something that models what happens at set-ui...

// Displayed
units = slippageIssuanceModule
  .callStatic
  .getRequiredComponentRedemptionUnitsOffChain(setToken, quantity);

// Real
slippageIssuanceModule.redeem(setToken, quantity)

// Test
Does displayed units match the redeem transaction outcome?

Does that make sense?

[0xSachinK]

@cgewecke Added in 9b88be4

[0xSachinK]

Edited PR description to track all fixes.
Rebased branch to master.

[cgewecke]

This is looking really good. I think the only remaining question is about the revert Iosiro triggered when depositing after a withdrawal. They suggested we airdrop into the Set (from Telegram):

WDYT about this and should we consider using Airdrop module or something to manage it?

EDIT - sorry now I see you've addressed this in the audit PR.

[cgewecke]

👍

[cgewecke]

LGTM! I guess we could merge here and rebase #232 against it in case additional integration tests would be helpful there.

[cgewecke]

🎉 This PR is included in version 0.9.0-hhat.1 🎉

The release is available on:

• npm package (@hardhat dist-tag)
• GitHub release

Your semantic-release bot 📦🚀