Skip to content

Commit 245702a

Browse files
shankars99shankars99
committed
audit patches + improvement (#1600)
Signed-off-by: shankar <shankar@layerzerolabs.org>
1 parent a683b9d commit 245702a

15 files changed

Lines changed: 563 additions & 683 deletions

‎examples/ovault-evm/contracts/MyOVaultComposer.sol‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,10 @@ pragma solidity ^0.8.22;
44
import { OVaultComposer } from "@layerzerolabs/ovault-evm/contracts/OVaultComposer.sol";
55

66
contract MyOVaultComposer is OVaultComposer {
7-
constructor(address _ovault, address _assetOFT, address _shareOFT) OVaultComposer(_ovault, _assetOFT, _shareOFT) {}
7+
constructor(
8+
address _ovault,
9+
address _assetOFT,
10+
address _shareOFT,
11+
address _refundOverpayAddress
12+
) OVaultComposer(_ovault, _assetOFT, _shareOFT, _refundOverpayAddress) {}
813
}

‎examples/ovault-evm/deploy/MyOVault.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const tokenSymbol = 'MockShare'
1111
const shareOFTAdapterContractName = 'MyShareOFTAdapter'
1212
const composerContractName = 'MyOVaultComposer'
1313

14+
const refundOverpayAddress = '0x0000000000000000000000000000000000000000'
15+
1416
const deploy: DeployFunction = async (hre) => {
1517
const { getNamedAccounts, deployments } = hre
1618

@@ -24,6 +26,10 @@ const deploy: DeployFunction = async (hre) => {
2426

2527
const assetOFTDeployment = await hre.deployments.get(assetOFTContractName)
2628

29+
if (refundOverpayAddress === '0x0000000000000000000000000000000000000000') {
30+
throw new Error('Refund overpay address is not set')
31+
}
32+
2733
const { address: ovaultAddress } = await deploy(ovaultContractName, {
2834
from: deployer,
2935
args: [tokenName, tokenSymbol, assetOFTDeployment.address],
@@ -48,7 +54,7 @@ const deploy: DeployFunction = async (hre) => {
4854

4955
const { address: composerAddress } = await deploy(composerContractName, {
5056
from: deployer,
51-
args: [ovaultAddress, assetOFTDeployment.address, shareOFTAdapterAddress],
57+
args: [ovaultAddress, assetOFTDeployment.address, shareOFTAdapterAddress, refundOverpayAddress],
5258
log: true,
5359
skipIfAlreadyDeployed: true,
5460
})

‎examples/ovault-evm/test/OVault_ERC4626_Equivalence.t.sol‎

Lines changed: 0 additions & 216 deletions
Original file line numberDiff line numberDiff line change
@@ -126,222 +126,6 @@ contract OVaultERC4626EquivalenceTest is TestHelperOz5 {
126126
assertEq(assetOFT.balanceOf(alice), alicePreDepositBal);
127127
}
128128

129-
function test_ovault_MultipleMintDepositRedeemWithdraw() public {
130-
// Scenario:
131-
// A = Alice, B = Bob
132-
// ________________________________________________________
133-
// | Vault shares | A share | A assets | B share | B assets |
134-
// |========================================================|
135-
// | 1. Alice mints 2000 shares (costs 2000 tokens) |
136-
// |--------------|---------|----------|---------|----------|
137-
// | 2000 | 2000 | 2000 | 0 | 0 |
138-
// |--------------|---------|----------|---------|----------|
139-
// | 2. Bob deposits 4000 tokens (mints 4000 shares) |
140-
// |--------------|---------|----------|---------|----------|
141-
// | 6000 | 2000 | 2000 | 4000 | 4000 |
142-
// |--------------|---------|----------|---------|----------|
143-
// | 3. Vault mutates by +3000 tokens... |
144-
// | (simulated yield returned from strategy)... |
145-
// |--------------|---------|----------|---------|----------|
146-
// | 6000 | 2000 | 3000 | 4000 | 6000 |
147-
// |--------------|---------|----------|---------|----------|
148-
// | 4. Alice deposits 2000 tokens (mints 1333 shares) |
149-
// |--------------|---------|----------|---------|----------|
150-
// | 7333 | 3333 | 4999 | 4000 | 6000 |
151-
// |--------------|---------|----------|---------|----------|
152-
// | 5. Bob mints 2000 shares (costs 3001 assets) |
153-
// | NOTE: Bob's assets spent got rounded up |
154-
// | NOTE: Alice's vault assets got rounded up |
155-
// |--------------|---------|----------|---------|----------|
156-
// | 9333 | 3333 | 5000 | 6000 | 9000 |
157-
// |--------------|---------|----------|---------|----------|
158-
// | 6. Vault mutates by +3000 tokens... |
159-
// | (simulated yield returned from strategy) |
160-
// | NOTE: Vault holds 17001 tokens, but sum of |
161-
// | assetsOf() is 17000. |
162-
// |--------------|---------|----------|---------|----------|
163-
// | 9333 | 3333 | 6071 | 6000 | 10929 |
164-
// |--------------|---------|----------|---------|----------|
165-
// | 7. Alice redeem 1333 shares (2428 assets) |
166-
// |--------------|---------|----------|---------|----------|
167-
// | 8000 | 2000 | 3643 | 6000 | 10929 |
168-
// |--------------|---------|----------|---------|----------|
169-
// | 8. Bob withdraws 2928 assets (1608 shares) |
170-
// |--------------|---------|----------|---------|----------|
171-
// | 6392 | 2000 | 3643 | 4392 | 8000 |
172-
// |--------------|---------|----------|---------|----------|
173-
// | 9. Alice withdraws 3643 assets (2000 shares) |
174-
// | NOTE: Bob's assets have been rounded back up |
175-
// |--------------|---------|----------|---------|----------|
176-
// | 4392 | 0 | 0 | 4392 | 8001 |
177-
// |--------------|---------|----------|---------|----------|
178-
// | 10. Bob redeem 4392 shares (8001 tokens) |
179-
// |--------------|---------|----------|---------|----------|
180-
// | 0 | 0 | 0 | 0 | 0 |
181-
// |______________|_________|__________|_________|__________|
182-
183-
address alice = address(0xABCD);
184-
address bob = address(0xDCBA);
185-
186-
uint256 mutationassetAmount = 3000;
187-
188-
assetOFT.mint(alice, 4000);
189-
190-
vm.prank(alice);
191-
assetOFT.approve(address(vault), 4000);
192-
193-
assertEq(assetOFT.allowance(alice, address(vault)), 4000);
194-
195-
assetOFT.mint(bob, 7001);
196-
197-
vm.prank(bob);
198-
assetOFT.approve(address(vault), 7001);
199-
200-
assertEq(assetOFT.allowance(bob, address(vault)), 7001);
201-
202-
// 1. Alice mints 2000 shares (costs 2000 tokens)
203-
vm.prank(alice);
204-
uint256 aliceassetAmount = vault.mint(2000, alice);
205-
206-
uint256 aliceShareAmount = vault.previewDeposit(aliceassetAmount);
207-
208-
// Expect to have received the requested mint amount.
209-
assertEq(aliceShareAmount, 2000);
210-
assertEq(vault.balanceOf(alice), aliceShareAmount);
211-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), aliceassetAmount);
212-
assertEq(vault.convertToShares(aliceassetAmount), vault.balanceOf(alice));
213-
214-
// Expect a 1:1 ratio before mutation.
215-
assertEq(aliceassetAmount, 2000);
216-
217-
// Sanity check.
218-
assertEq(vault.totalSupply(), aliceShareAmount);
219-
assertEq(vault.totalAssets(), aliceassetAmount);
220-
221-
// 2. Bob deposits 4000 tokens (mints 4000 shares)
222-
vm.prank(bob);
223-
uint256 bobShareAmount = vault.deposit(4000, bob);
224-
uint256 bobassetAmount = vault.previewWithdraw(bobShareAmount);
225-
226-
// Expect to have received the requested asset amount.
227-
assertEq(bobassetAmount, 4000);
228-
assertEq(vault.balanceOf(bob), bobShareAmount);
229-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), bobassetAmount);
230-
assertEq(vault.convertToShares(bobassetAmount), vault.balanceOf(bob));
231-
232-
// Expect a 1:1 ratio before mutation.
233-
assertEq(bobShareAmount, bobassetAmount);
234-
235-
// Sanity check.
236-
uint256 preMutationShareBal = aliceShareAmount + bobShareAmount;
237-
uint256 preMutationBal = aliceassetAmount + bobassetAmount;
238-
assertEq(vault.totalSupply(), preMutationShareBal);
239-
assertEq(vault.totalAssets(), preMutationBal);
240-
assertEq(vault.totalSupply(), 6000);
241-
assertEq(vault.totalAssets(), 6000);
242-
243-
// 3. Vault mutates by +3000 tokens... |
244-
// (simulated yield returned from strategy)...
245-
// The Vault now contains more tokens than deposited which causes the exchange rate to change.
246-
// Alice share is 33.33% of the Vault, Bob 66.66% of the Vault.
247-
// Alice's share count stays the same but the asset amount changes from 2000 to 3000.
248-
// Bob's share count stays the same but the asset amount changes from 4000 to 6000.
249-
assetOFT.mint(address(vault), mutationassetAmount);
250-
assertEq(vault.totalSupply(), preMutationShareBal);
251-
assertEq(vault.totalAssets(), preMutationBal + mutationassetAmount);
252-
assertEq(vault.balanceOf(alice), aliceShareAmount);
253-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), aliceassetAmount + (mutationassetAmount / 3) * 1);
254-
assertEq(vault.balanceOf(bob), bobShareAmount);
255-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), bobassetAmount + (mutationassetAmount / 3) * 2);
256-
257-
// 4. Alice deposits 2000 tokens (mints 1333 shares)
258-
vm.prank(alice);
259-
vault.deposit(2000, alice);
260-
261-
assertEq(vault.totalSupply(), 7333);
262-
assertEq(vault.balanceOf(alice), 3333);
263-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 4999);
264-
assertEq(vault.balanceOf(bob), 4000);
265-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 6000);
266-
267-
// 5. Bob mints 2000 shares (costs 3001 assets)
268-
// NOTE: Bob's assets spent got rounded up
269-
// NOTE: Alices's vault assets got rounded up
270-
vm.prank(bob);
271-
vault.mint(2000, bob);
272-
273-
assertEq(vault.totalSupply(), 9333);
274-
assertEq(vault.balanceOf(alice), 3333);
275-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 5000);
276-
assertEq(vault.balanceOf(bob), 6000);
277-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 9000);
278-
279-
// Sanity checks:
280-
// Alice and bob should have spent all their tokens now
281-
assertEq(assetOFT.balanceOf(alice), 0);
282-
assertEq(assetOFT.balanceOf(bob), 0);
283-
// Assets in vault: 4k (alice) + 7k (bob) + 3k (yield) + 1 (round up)
284-
assertEq(vault.totalAssets(), 14001);
285-
286-
// 6. Vault mutates by +3000 tokens
287-
// NOTE: Vault holds 17001 tokens, but sum of assetsOf() is 17000.
288-
assetOFT.mint(address(vault), mutationassetAmount);
289-
assertEq(vault.totalAssets(), 17001);
290-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 6071);
291-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 10929);
292-
293-
// 7. Alice redeem 1333 shares (2428 assets)
294-
vm.prank(alice);
295-
vault.redeem(1333, alice, alice);
296-
297-
assertEq(assetOFT.balanceOf(alice), 2428);
298-
assertEq(vault.totalSupply(), 8000);
299-
assertEq(vault.totalAssets(), 14573);
300-
assertEq(vault.balanceOf(alice), 2000);
301-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 3643);
302-
assertEq(vault.balanceOf(bob), 6000);
303-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 10929);
304-
305-
// 8. Bob withdraws 2929 assets (1608 shares)
306-
vm.prank(bob);
307-
vault.withdraw(2929, bob, bob);
308-
309-
assertEq(assetOFT.balanceOf(bob), 2929);
310-
assertEq(vault.totalSupply(), 6392);
311-
assertEq(vault.totalAssets(), 11644);
312-
assertEq(vault.balanceOf(alice), 2000);
313-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 3643);
314-
assertEq(vault.balanceOf(bob), 4392);
315-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 8000);
316-
317-
// 9. Alice withdraws 3643 assets (2000 shares)
318-
// NOTE: Bob's assets have been rounded back up
319-
vm.prank(alice);
320-
vault.withdraw(3643, alice, alice);
321-
322-
assertEq(assetOFT.balanceOf(alice), 6071);
323-
assertEq(vault.totalSupply(), 4392);
324-
assertEq(vault.totalAssets(), 8001);
325-
assertEq(vault.balanceOf(alice), 0);
326-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 0);
327-
assertEq(vault.balanceOf(bob), 4392);
328-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 8001);
329-
330-
// 10. Bob redeem 4392 shares (8001 tokens)
331-
vm.prank(bob);
332-
vault.redeem(4392, bob, bob);
333-
assertEq(assetOFT.balanceOf(bob), 10930);
334-
assertEq(vault.totalSupply(), 0);
335-
assertEq(vault.totalAssets(), 0);
336-
assertEq(vault.balanceOf(alice), 0);
337-
assertEq(vault.convertToAssets(vault.balanceOf(alice)), 0);
338-
assertEq(vault.balanceOf(bob), 0);
339-
assertEq(vault.convertToAssets(vault.balanceOf(bob)), 0);
340-
341-
// Sanity check
342-
assertEq(assetOFT.balanceOf(address(vault)), 0);
343-
}
344-
345129
function test_ovault_FailDepositWithNotEnoughApproval() public {
346130
assetOFT.mint(address(this), 0.5e18);
347131
assetOFT.approve(address(vault), 0.5e18);

‎examples/ovault-evm/test/composer/OVaultComposer_Base.t.sol‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { OptionsBuilder } from "@layerzerolabs/oapp-evm/contracts/oapp/libs/Opti
77
// OFT imports
88
import { OFTComposeMsgCodec } from "@layerzerolabs/oft-evm/contracts/libs/OFTComposeMsgCodec.sol";
99
import { SendParam, MessagingFee } from "@layerzerolabs/oft-evm/contracts/interfaces/IOFT.sol";
10+
import { EnforcedOptionParam } from "@layerzerolabs/oapp-evm/contracts/oapp/interfaces/IOAppOptionsType3.sol";
1011

1112
import { OVaultComposer } from "@layerzerolabs/ovault-evm/contracts/OVaultComposer.sol";
1213

@@ -45,9 +46,11 @@ contract OVaultComposerBaseTest is TestHelperOz5 {
4546
address public userA = makeAddr("userA");
4647
address public userB = makeAddr("userB");
4748

49+
address public refundOverpayAddress = makeAddr("refundOverpayAddress");
50+
4851
address public arbEndpoint;
4952
address public arbExecutor = makeAddr("arbExecutor");
50-
bytes public OPTIONS_LZRECEIVE_2M = OptionsBuilder.newOptions().addExecutorLzReceiveOption(200_000, 0);
53+
bytes public OPTIONS_LZRECEIVE_100k = OptionsBuilder.newOptions().addExecutorLzReceiveOption(100_000, 0);
5154

5255
uint256 public constant INITIAL_BALANCE = 100 ether;
5356
uint256 public constant TOKENS_TO_SEND = 1 ether;
@@ -73,7 +76,12 @@ contract OVaultComposerBaseTest is TestHelperOz5 {
7376
/// Now the "expansion" is for the arb vault and share ofts on other networks.
7477
oVault_arb = new MockOVault("arbShare", "arbShare", address(assetOFT_arb));
7578
shareOFT_arb = new MockOFTAdapter(address(oVault_arb), address(endpoints[ARB_EID]), address(this));
76-
OVaultComposerArb = new OVaultComposer(address(oVault_arb), address(assetOFT_arb), address(shareOFT_arb));
79+
OVaultComposerArb = new OVaultComposer(
80+
address(oVault_arb),
81+
address(assetOFT_arb),
82+
address(shareOFT_arb),
83+
refundOverpayAddress
84+
);
7785

7886
/// Deploy the Share OFTs on other networks - these are NOT lockbox adapters.
7987
shareOFT_eth = new MockOFT("ethShare", "ethShare", address(endpoints[ETH_EID]), address(this));
@@ -98,6 +106,13 @@ contract OVaultComposerBaseTest is TestHelperOz5 {
98106

99107
deal(arbExecutor, INITIAL_BALANCE);
100108
deal(arbEndpoint, INITIAL_BALANCE);
109+
110+
EnforcedOptionParam[] memory enforcedOptions = new EnforcedOptionParam[](2);
111+
enforcedOptions[0] = EnforcedOptionParam({ eid: ETH_EID, msgType: 1, options: OPTIONS_LZRECEIVE_100k });
112+
enforcedOptions[1] = EnforcedOptionParam({ eid: POL_EID, msgType: 1, options: OPTIONS_LZRECEIVE_100k });
113+
114+
assetOFT_arb.setEnforcedOptions(enforcedOptions);
115+
shareOFT_arb.setEnforcedOptions(enforcedOptions);
101116
}
102117

103118
function _createComposePayload(
@@ -139,6 +154,25 @@ contract OVaultComposerBaseTest is TestHelperOz5 {
139154
assetOFT_arb.mint(address(oVault_arb), mintAssets);
140155
}
141156

157+
function _removeDustWithOffset(uint256 _amount, int128 _offset) internal pure returns (uint256, uint256) {
158+
uint256 amountWithOffset = _amount;
159+
if (_offset < 0) {
160+
uint256 modOffset = uint128(-1 * _offset);
161+
// If offset is negative, we need to ensure we don't underflow
162+
require(_amount >= modOffset, "Offset too large");
163+
amountWithOffset = _amount - modOffset;
164+
} else {
165+
// If offset is positive, we can safely add it
166+
amountWithOffset = _amount + uint128(_offset);
167+
}
168+
return _removeDust(amountWithOffset);
169+
}
170+
171+
function _removeDust(uint256 _amount) internal pure returns (uint256, uint256) {
172+
uint256 dust = _amount % 10 ** 12;
173+
return (_amount - dust, dust);
174+
}
175+
142176
function _randomGUID() internal view returns (bytes32) {
143177
return bytes32(vm.randomBytes(32));
144178
}

‎examples/ovault-evm/test/composer/OVaultComposer_E2E.t.sol‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ contract OVaultComposerE2ETest is OVaultComposerBaseTest {
3636
}
3737

3838
function test_E2E_ethereum_to_polygon() public {
39-
uint256 shareTokensToReceive = TOKENS_TO_SEND * 2;
39+
(uint256 shareTokensToReceive, ) = _removeDustWithOffset(TOKENS_TO_SEND * 2, -1);
4040

4141
deal(address(assetOFT_eth), userA, TOKENS_TO_SEND);
4242

@@ -105,10 +105,11 @@ contract OVaultComposerE2ETest is OVaultComposerBaseTest {
105105
address(this),
106106
""
107107
);
108+
108109
assertEq(
109110
assetOFT_arb.balanceOf(composerAddress),
110111
0,
111-
"composerAddress should have no tokens after lzCompose on arb"
112+
"composerAddress should have the no tokens after lzCompose on arb"
112113
);
113114

114115
verifyPackets(POL_EID, addressToBytes32(address(shareOFT_pol)));

0 commit comments

Comments
 (0)