Skip to content

Commit 46f165e

Browse files
shankars99shankars99
committed
ovault composer audit resolution (#1639)
Signed-off-by: shankar <shankar@layerzerolabs.org>
1 parent ef90208 commit 46f165e

4 files changed

Lines changed: 50 additions & 23 deletions

File tree

‎packages/ovault-evm/contracts/VaultComposerSync.sol‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,10 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
7676
/// @dev burn() on tokens when a user sends changes totalSupply() which the asset:share ratio depends on.
7777
if (!IOFT(SHARE_OFT).approvalRequired()) revert ShareOFTNotAdapter(SHARE_OFT);
7878

79-
/// @dev Approve the vault to spend the share and asset tokens held by this contract
80-
IERC20(SHARE_ERC20).approve(_vault, type(uint256).max);
79+
/// @dev Approve the vault to spend the asset tokens held by this contract
8180
IERC20(ASSET_ERC20).approve(_vault, type(uint256).max);
81+
/// @dev Approving the vault for the share erc20 is not required when the vault is the share erc20
82+
// IERC20(SHARE_ERC20).approve(_vault, type(uint256).max);
8283

8384
/// @dev Approve the share adapter with the share tokens held by this contract
8485
IERC20(SHARE_ERC20).approve(_shareOFT, type(uint256).max);
@@ -90,26 +91,26 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
9091
* @notice Handles LayerZero compose operations for vault transactions with automatic refund functionality
9192
* @dev This composer is designed to handle refunds to an EOA address and not a contract
9293
* @dev Any revert in handleCompose() causes a refund back to the src EXCEPT for InsufficientMsgValue
93-
* @param _composeCaller The OFT contract address used for refunds, must be either ASSET_OFT or SHARE_OFT
94+
* @param _composeSender The OFT contract address used for refunds, must be either ASSET_OFT or SHARE_OFT
9495
* @param _guid LayerZero's unique tx id (created on the source tx)
9596
* @param _message Decomposable bytes object into [composeHeader][composeMessage]
9697
*/
9798
function lzCompose(
98-
address _composeCaller, // The OFT used on refund, also the vaultIn token.
99+
address _composeSender, // The OFT used on refund, also the vaultIn token.
99100
bytes32 _guid,
100101
bytes calldata _message, // expected to contain a composeMessage = abi.encode(SendParam hopSendParam,uint256 minMsgValue)
101102
address /*_executor*/,
102103
bytes calldata /*_extraData*/
103104
) external payable virtual override {
104105
if (msg.sender != ENDPOINT) revert OnlyEndpoint(msg.sender);
105-
if (_composeCaller != ASSET_OFT && _composeCaller != SHARE_OFT) revert OnlyValidComposeCaller(_composeCaller);
106+
if (_composeSender != ASSET_OFT && _composeSender != SHARE_OFT) revert OnlyValidComposeCaller(_composeSender);
106107

107108
bytes32 composeFrom = _message.composeFrom();
108109
uint256 amount = _message.amountLD();
109110
bytes memory composeMsg = _message.composeMsg();
110111

111112
/// @dev try...catch to handle the compose operation. if it fails we refund the user
112-
try this.handleCompose{ value: msg.value }(_composeCaller, composeFrom, composeMsg, amount) {
113+
try this.handleCompose{ value: msg.value }(_composeSender, composeFrom, composeMsg, amount) {
113114
emit Sent(_guid);
114115
} catch (bytes memory _err) {
115116
/// @dev A revert where the msg.value passed is lower than the min expected msg.value is handled separately
@@ -120,7 +121,7 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
120121
}
121122
}
122123

123-
_refund(_composeCaller, _message, amount, tx.origin);
124+
_refund(_composeSender, _message, amount, tx.origin);
124125
emit Refunded(_guid);
125126
}
126127
}
@@ -272,23 +273,25 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
272273
/**
273274
* @notice Quotes the send operation for the given OFT and SendParam
274275
* @dev Revert on slippage will be thrown by the OFT and not _assertSlippage
275-
* @param _oft The OFT contract address to quote
276+
* @param _targetOFT The OFT contract address to quote
276277
* @param _vaultInAmount The amount of tokens to send to the vault
277278
* @param _sendParam The parameters for the send operation
278279
* @return MessagingFee The estimated fee for the send operation
279280
* @dev This function can be overridden to implement custom quoting logic
280281
*/
281282
function quoteSend(
282-
address _oft,
283+
address _targetOFT,
283284
uint256 _vaultInAmount,
284285
SendParam memory _sendParam
285286
) external view virtual returns (MessagingFee memory) {
286-
if (_oft == ASSET_OFT) {
287-
_sendParam.amountLD = VAULT.previewDeposit(_vaultInAmount);
288-
} else {
287+
/// @dev When quoting the asset OFT, the function input is shares and the SendParam.amountLD into quoteSend() should be assets (and vice versa)
288+
289+
if (_targetOFT == ASSET_OFT) {
289290
_sendParam.amountLD = VAULT.previewRedeem(_vaultInAmount);
291+
} else {
292+
_sendParam.amountLD = VAULT.previewDeposit(_vaultInAmount);
290293
}
291-
return IOFT(_oft).quoteSend(_sendParam, false);
294+
return IOFT(_targetOFT).quoteSend(_sendParam, false);
292295
}
293296

294297
/**
@@ -304,7 +307,7 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
304307
/// @dev Can do this because _oft is validated before this function is called
305308
address erc20 = _oft == ASSET_OFT ? ASSET_ERC20 : SHARE_ERC20;
306309

307-
if (msg.value > 0) revert InsufficientMsgValue(0, msg.value);
310+
if (msg.value > 0) revert NoMsgValueExpected();
308311
IERC20(erc20).safeTransfer(_sendParam.to.bytes32ToAddress(), _sendParam.amountLD);
309312
} else {
310313
// crosschain send

‎packages/ovault-evm/contracts/interfaces/IVaultComposerSync.sol‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ interface IVaultComposerSync is IOAppComposer {
2121
error OnlyValidComposeCaller(address caller); // 0x84fb3f0d
2222

2323
error InsufficientMsgValue(uint256 expectedMsgValue, uint256 actualMsgValue); // 0x7cb769dc
24+
error NoMsgValueExpected(); // 0x7578d2bd
2425

2526
error SlippageExceeded(uint256 amountLD, uint256 minAmountLD); // 0x71c4efed
2627

@@ -55,14 +56,14 @@ interface IVaultComposerSync is IOAppComposer {
5556

5657
/**
5758
* @notice Quotes the send operation for the given OFT and SendParam
58-
* @param oft The OFT contract address to quote
59+
* @param targetOft The OFT contract address to quote
5960
* @param vaultInAmount The amount of tokens to send to the vault
6061
* @param sendParam The parameters for the send operation
6162
* @return MessagingFee The estimated fee for the send operation
6263
* @dev This function can be overridden to implement custom quoting logic
6364
*/
6465
function quoteSend(
65-
address oft,
66+
address targetOft,
6667
uint256 vaultInAmount,
6768
SendParam memory sendParam
6869
) external view returns (MessagingFee memory);

‎packages/ovault-evm/test/vault-sync/VaultComposerSync_ProxySend.t.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ contract VaultComposerSyncProxySendTest is VaultComposerSyncBaseTest {
3232
assetOFT_arb.approve(address(VaultComposerSyncArb), TOKENS_TO_SEND);
3333
vault_arb.approve(address(VaultComposerSyncArb), TOKENS_TO_SEND);
3434

35-
vm.expectRevert(abi.encodeWithSelector(IVaultComposerSync.InsufficientMsgValue.selector, 0, 1));
35+
vm.expectRevert(IVaultComposerSync.NoMsgValueExpected.selector);
3636
VaultComposerSyncArb.depositAndSend{ value: 1 wei }(TOKENS_TO_SEND, sendParam, userA);
3737

38-
vm.expectRevert(abi.encodeWithSelector(IVaultComposerSync.InsufficientMsgValue.selector, 0, 2));
38+
vm.expectRevert(IVaultComposerSync.NoMsgValueExpected.selector);
3939
VaultComposerSyncArb.redeemAndSend{ value: 2 wei }(TOKENS_TO_SEND, sendParam, userA);
4040
vm.stopPrank();
4141
}

‎packages/ovault-evm/test/vault-sync/VaultComposerSync_Unit.t.sol‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,12 @@ contract VaultComposerSyncUnitTest is VaultComposerSyncBaseTest {
5757
emit IERC20.Transfer(address(0), address(VaultComposerSyncArb), TOKENS_TO_SEND);
5858

5959
vm.expectEmit(true, true, true, true, address(vault_arb));
60-
emit IERC4626.Deposit(address(VaultComposerSyncArb), address(VaultComposerSyncArb), TOKENS_TO_SEND, TOKENS_TO_SEND);
60+
emit IERC4626.Deposit(
61+
address(VaultComposerSyncArb),
62+
address(VaultComposerSyncArb),
63+
TOKENS_TO_SEND,
64+
TOKENS_TO_SEND
65+
);
6166

6267
vm.expectEmit(true, true, true, true, address(VaultComposerSyncArb));
6368
emit IVaultComposerSync.Sent(guid);
@@ -95,7 +100,12 @@ contract VaultComposerSyncUnitTest is VaultComposerSyncBaseTest {
95100
emit IERC20.Transfer(address(0), address(VaultComposerSyncArb), TOKENS_TO_SEND);
96101

97102
vm.expectEmit(true, true, true, true, address(vault_arb));
98-
emit IERC4626.Deposit(address(VaultComposerSyncArb), address(VaultComposerSyncArb), TOKENS_TO_SEND, TOKENS_TO_SEND);
103+
emit IERC4626.Deposit(
104+
address(VaultComposerSyncArb),
105+
address(VaultComposerSyncArb),
106+
TOKENS_TO_SEND,
107+
TOKENS_TO_SEND
108+
);
99109

100110
vm.expectEmit(true, true, true, true, address(VaultComposerSyncArb));
101111
emit IVaultComposerSync.Sent(guid);
@@ -110,9 +120,10 @@ contract VaultComposerSyncUnitTest is VaultComposerSyncBaseTest {
110120
assertEq(vault_arb.totalSupply(), vault_arb.balanceOf(address(userA)), TOKENS_TO_SEND);
111121
}
112122

113-
function test_lzCompose_pass_dst_is_hub_no_msgValue() public {
123+
function test_lzCompose_pass_dst_is_hub_no_msgValue_causes_refund() public {
114124
bytes32 guid = _randomGUID();
115125
assetOFT_arb.mint(address(VaultComposerSyncArb), TOKENS_TO_SEND);
126+
uint256 userABalanceEth = assetOFT_eth.balanceOf(userA);
116127

117128
SendParam memory internalSendParam = SendParam(
118129
VaultComposerSyncArb.VAULT_EID(),
@@ -126,9 +137,21 @@ contract VaultComposerSyncUnitTest is VaultComposerSyncBaseTest {
126137

127138
bytes memory composeMsg = _createComposePayload(ETH_EID, internalSendParam, 1 wei, TOKENS_TO_SEND, userA);
128139

129-
vm.expectRevert(abi.encodeWithSelector(IVaultComposerSync.InsufficientMsgValue.selector, 0, 1));
140+
/// @dev Internal revert on try...catch
141+
/// vm.expectRevert(abi.encodeWithSelector(IVaultComposerSync.NoMsgValueExpected.selector));
142+
143+
vm.expectEmit(true, true, true, true, address(VaultComposerSyncArb));
144+
emit IVaultComposerSync.Refunded(guid);
145+
130146
vm.prank(arbEndpoint);
131-
VaultComposerSyncArb.lzCompose{ value: 1 wei }(address(assetOFT_arb), guid, composeMsg, arbExecutor, "");
147+
VaultComposerSyncArb.lzCompose{ value: 1 ether }(address(assetOFT_arb), guid, composeMsg, arbExecutor, "");
148+
149+
verifyPackets(ETH_EID, address(assetOFT_eth));
150+
assertEq(
151+
assetOFT_eth.balanceOf(userA),
152+
userABalanceEth + TOKENS_TO_SEND,
153+
"userA should have received refund on Ethereum"
154+
);
132155
}
133156

134157
function test_lzCompose_fail_invalid_payload_auto_refunds() public {

0 commit comments

Comments
 (0)