Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(contracts): add msg.Value to verifyMessageId #4541

Merged
merged 59 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
1560f5c
add fix
aroralanuk Sep 5, 2024
44379e1
move to inside
aroralanuk Sep 13, 2024
d680e50
opl2tol1
aroralanuk Sep 16, 2024
ca26e88
arb
aroralanuk Sep 16, 2024
a5c4b62
rm revertWhen_invalidMetadata
aroralanuk Sep 16, 2024
54b4327
msg_value
aroralanuk Sep 16, 2024
66c3bb7
direct call
aroralanuk Sep 16, 2024
e98382d
invalid ism
aroralanuk Sep 16, 2024
56ea53c
no both branch test
aroralanuk Sep 16, 2024
30982d2
unauth hook
aroralanuk Sep 17, 2024
88cb33f
invalid messageId
aroralanuk Sep 17, 2024
baef8ff
5164
aroralanuk Sep 17, 2024
9684835
try catch
aroralanuk Sep 17, 2024
6c31fbf
formatting
aroralanuk Sep 17, 2024
f64611d
opstack
aroralanuk Sep 18, 2024
ac3613e
minor edits
aroralanuk Sep 18, 2024
4d293ea
minor fixes
aroralanuk Sep 18, 2024
fb66306
override inconsistent tests
aroralanuk Sep 18, 2024
13c61fd
Merge branch 'kunal/external-bridge-refactor' into kunal/HL2408-002-fix
aroralanuk Sep 20, 2024
fb089e2
opstack change quote
aroralanuk Sep 20, 2024
c70ac54
Merge branch 'kunal/external-bridge-refactor' into kunal/HL2408-002-fix
aroralanuk Sep 20, 2024
e6c5b10
fix tests
aroralanuk Sep 20, 2024
08dda88
remove reduntant arbitrary call
aroralanuk Sep 20, 2024
37ecb27
rm
aroralanuk Sep 20, 2024
1dd0fc3
changeset
aroralanuk Sep 20, 2024
c99eb24
add verifyMessageId
aroralanuk Sep 20, 2024
97b7bc8
add test
aroralanuk Sep 20, 2024
c6b5f76
Merge branch 'main' into kunal/verify-message-id
aroralanuk Sep 20, 2024
bef3470
add to current value
aroralanuk Sep 20, 2024
541be43
revert
aroralanuk Sep 20, 2024
4233c0a
magic
aroralanuk Sep 20, 2024
41a407d
5164
aroralanuk Sep 20, 2024
5610c33
changeset
aroralanuk Sep 20, 2024
ef780c6
check sufficient fees and return extra
aroralanuk Sep 23, 2024
a6731b0
changeset
aroralanuk Sep 23, 2024
47defbf
Merge branch 'main' into kunal/HL2408-002-fix
aroralanuk Sep 24, 2024
8510b6a
add childhook
aroralanuk Sep 24, 2024
a1a850e
add tests
aroralanuk Sep 24, 2024
7ba121d
inter
aroralanuk Sep 25, 2024
101fe72
Merge branch 'kunal/check-suff-refund-extra' into kunal/verify-messag…
aroralanuk Sep 25, 2024
cc69d2e
hook encode fix
aroralanuk Sep 25, 2024
a0aaf23
add msgvalue to quote
aroralanuk Sep 25, 2024
5d90cfe
Merge branch 'kunal/check-suff-refund-extra' into kunal/verify-messag…
aroralanuk Sep 25, 2024
43eba79
revert
aroralanuk Sep 25, 2024
b10361e
verify fixes
aroralanuk Sep 25, 2024
66bf788
Merge branch 'kunal/HL2408-002-fix' into kunal/verify-message-id
aroralanuk Sep 25, 2024
9851f10
test
aroralanuk Sep 26, 2024
750d09b
spelling
aroralanuk Sep 26, 2024
5991022
Merge branch 'main' into kunal/HL2408-002-fix
aroralanuk Sep 26, 2024
7028f14
Merge branch 'kunal/HL2408-002-fix' into kunal/verify-message-id
aroralanuk Sep 26, 2024
71d23f4
Merge branch 'kunal/HL2408-002-fix' into kunal/check-suff-refund-extra
aroralanuk Sep 26, 2024
8df9105
rm changeset
aroralanuk Sep 26, 2024
fd62639
Merge branch 'kunal/check-suff-refund-extra' into kunal/verify-messag…
aroralanuk Sep 26, 2024
c9e1fa7
spell
aroralanuk Sep 26, 2024
562fcd4
preverifyMessage
aroralanuk Oct 4, 2024
c604337
Merge branch 'main' into kunal/verify-message-id
aroralanuk Oct 31, 2024
89b001d
docs(changeset): Added msg.value to preverifyMessage to commit it as …
aroralanuk Oct 31, 2024
2150154
changeset
aroralanuk Oct 31, 2024
f4f638e
revert
aroralanuk Oct 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'main' into kunal/verify-message-id
  • Loading branch information
aroralanuk committed Sep 20, 2024
commit c6b5f763e773a257c7d490a8755b846ae38b12d9
6 changes: 3 additions & 3 deletions solidity/test/isms/ERC5164ISM.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ contract ERC5164IsmTest is ExternalBridgeTest {
vm.expectRevert(
"AbstractMessageIdAuthorizedIsm: sender is not the hook"
);
ism.verifyMessageId(messageId, 0);
ism.verifyMessageId(messageId);
assertFalse(ism.isVerified(encodedMessage));
}

Expand All @@ -157,7 +157,7 @@ contract ERC5164IsmTest is ExternalBridgeTest {
uint256 _msgValue
) internal override {
vm.prank(address(executor));
ism.verifyMessageId(messageId, _msgValue);
ism.verifyMessageId(messageId);
}

function _encodeExternalDestinationBridgeCall(
Expand All @@ -168,7 +168,7 @@ contract ERC5164IsmTest is ExternalBridgeTest {
) internal override returns (bytes memory) {
if (_from == address(hook)) {
vm.prank(address(executor));
ism.verifyMessageId{value: _msgValue}(messageId, _msgValue);
ism.verifyMessageId{value: _msgValue}(messageId);
}
}
}
24 changes: 11 additions & 13 deletions solidity/test/isms/ExternalBridgeTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ abstract contract ExternalBridgeTest is Test {
/* ============ Hook.postDispatch ============ */

function test_postDispatch() public {
bytes memory encodedHookData = _encodeHookData(messageId, 0);
bytes memory encodedHookData = _encodeHookData(messageId);
originMailbox.updateLatestDispatchedId(messageId);
_expectOriginExternalBridgeCall(encodedHookData);

Expand Down Expand Up @@ -93,7 +93,10 @@ abstract contract ExternalBridgeTest is Test {
/* ============ ISM.verifyMessageId ============ */

function test_verifyMessageId_asyncCall() public {
bytes memory encodedHookData = _encodeHookData(messageId, 0);
bytes memory encodedHookData = abi.encodeCall(
AbstractMessageIdAuthorizedIsm.verifyMessageId,
(messageId)
);
_externalBridgeDestinationCall(encodedHookData, 0);

assertTrue(ism.isVerified(encodedMessage));
Expand All @@ -119,7 +122,7 @@ abstract contract ExternalBridgeTest is Test {
}

function test_verify_msgValue_asyncCall() public virtual {
bytes memory encodedHookData = _encodeHookData(messageId, MSG_VALUE);
bytes memory encodedHookData = _encodeHookData(messageId);
_externalBridgeDestinationCall(encodedHookData, MSG_VALUE);

assertTrue(ism.verify(new bytes(0), encodedMessage));
Expand Down Expand Up @@ -176,7 +179,7 @@ abstract contract ExternalBridgeTest is Test {
bytes memory externalCalldata = _encodeExternalDestinationBridgeCall(
address(hook),
address(ism),
MSG_VALUE,
0,
incorrectMessageId
);

Expand All @@ -187,21 +190,17 @@ abstract contract ExternalBridgeTest is Test {
// async call - native bridges might have try catch block to prevent revert
try
this.externalBridgeDestinationCallWrapper(
_encodeHookData(incorrectMessageId, MSG_VALUE),
_encodeHookData(incorrectMessageId),
0
)
{} catch {}
assertFalse(ism.isVerified(testMessage));
assertEq(address(testRecipient).balance, 0);
}

/// forge-config: default.fuzz.runs = 10
function test_verify_valueAlreadyClaimed(uint256 _msgValue) public virtual {
_msgValue = bound(_msgValue, 0, MAX_MSG_VALUE);
_externalBridgeDestinationCall(
_encodeHookData(messageId, _msgValue),
_msgValue
);
_externalBridgeDestinationCall(_encodeHookData(messageId), _msgValue);

bool verified = ism.verify(new bytes(0), encodedMessage);
assertTrue(verified);
Expand Down Expand Up @@ -263,13 +262,12 @@ abstract contract ExternalBridgeTest is Test {
}

function _encodeHookData(
bytes32 _messageId,
uint256 _msgValue
bytes32 _messageId
) internal pure returns (bytes memory) {
return
abi.encodeCall(
AbstractMessageIdAuthorizedIsm.verifyMessageId,
(_messageId, _msgValue)
(_messageId)
);
}

Expand Down
2 changes: 1 addition & 1 deletion solidity/test/isms/OPL2ToL1Ism.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest {
uint256 _value,
bytes32 _messageId
) internal view returns (bytes memory) {
bytes memory encodedHookData = _encodeHookData(_messageId, _value);
bytes memory encodedHookData = _encodeHookData(_messageId);

return
abi.encodeCall(
Expand Down
15 changes: 4 additions & 11 deletions solidity/test/isms/OPStackIsm.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,8 @@ contract OPStackIsmTest is ExternalBridgeTest {
function test_verify_revertsWhen_incorrectMessageId() public override {
bytes32 incorrectMessageId = keccak256("incorrect message id");

_externalBridgeDestinationCall(
_encodeHookData(incorrectMessageId, MSG_VALUE),
0
);
_externalBridgeDestinationCall(_encodeHookData(incorrectMessageId), 0);
assertFalse(ism.isVerified(testMessage));
assertEq(address(testRecipient).balance, 0);
}

/* ============ helper functions ============ */
Expand Down Expand Up @@ -157,7 +153,7 @@ contract OPStackIsmTest is ExternalBridgeTest {
function test_verify_revertsWhen_notAuthorizedHook() public override {
// needs to be called by the canonical messenger on Optimism
vm.expectRevert(NotCrossChainCall.selector);
ism.verifyMessageId(messageId, 0);
ism.verifyMessageId(messageId);

vm.startPrank(L2_MESSENGER_ADDRESS);
_setExternalOriginSender(address(this));
Expand All @@ -166,7 +162,7 @@ contract OPStackIsmTest is ExternalBridgeTest {
vm.expectRevert(
"AbstractMessageIdAuthorizedIsm: sender is not the hook"
);
ism.verifyMessageId(messageId, 0);
ism.verifyMessageId(messageId);
}

function _setExternalOriginSender(
Expand All @@ -184,10 +180,7 @@ contract OPStackIsmTest is ExternalBridgeTest {
vm.expectRevert(
"AbstractMessageIdAuthorizedIsm: msg.value must be less than 2^255"
);
_externalBridgeDestinationCall(
_encodeHookData(messageId, _msgValue),
_msgValue
);
_externalBridgeDestinationCall(_encodeHookData(messageId), _msgValue);

assertFalse(ism.isVerified(encodedMessage));

Expand Down
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.