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.

Sign up for GitHub

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

feat(protocol): add back EventProcessed event and improve gas logging in Bridge #16760

Merged
merged 9 commits into from
Apr 19, 2024
Merged

feat(protocol): add back EventProcessed event and improve gas logging in Bridge #16760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c8a8580
add an event and improve gas logging
dantaik Apr 18, 2024
3fc30f4
Update Bridge.sol
dantaik Apr 18, 2024
4e90f86
Update Bridge.sol
dantaik Apr 18, 2024
1288379
Update Bridge.sol
dantaik Apr 19, 2024
f37c195
Update packages/protocol/contracts/bridge/Bridge.sol
dantaik Apr 19, 2024
7dc31f8
Update packages/protocol/contracts/bridge/Bridge.sol
dantaik Apr 19, 2024
4a27603
Merge branch 'bridge_process_event' of https://github.com/taikoxyz/ta…
dantaik Apr 19, 2024
8f346c6
fix tests
dantaik Apr 19, 2024
a5630e1
Merge branch 'main' into bridge_process_event
dantaik Apr 19, 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
41 changes: 30 additions & 11 deletions packages/protocol/contracts/bridge/Bridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,26 @@ contract Bridge is EssentialContract, IBridge {
using LibAddress for address;
using LibAddress for address payable;

/// @dev An event for fine-tune gas related constants in the future.
struct GasStats {
uint256 start;
uint256 beforeInvocation;
uint256 afterInvocation;
uint256 gasUsedInFeeCalc;
uint256 end;
}

/// @dev A debug event for fine-tuning gas related constants in the future.
event GasLog(
uint256 indexed messageId,
uint256 gasMeasured,
bytes32 indexed msgHash,
GasStats stats,
uint256 overhead,
uint256 proofSize,
uint256 numCacheOps
);

/// @dev The amount of gas that will be deducted from message.gasLimit before calculating the
/// invocation gas limit. This value should be fine-tuned with production data.
uint32 public constant GAS_RESERVE = 250_000;
uint32 public constant GAS_RESERVE = 650_000;

/// @dev The gas overhead for both receiving and invoking a message, as well as the proof
/// calldata cost.
Expand Down Expand Up @@ -198,6 +206,9 @@ contract Bridge is EssentialContract, IBridge {
}

/// @inheritdoc IBridge
/// @dev This transaction's gas limit must not be smaller than:
/// `(message.gasLimit - GAS_RESERVE) * 64 / 63 + GAS_RESERVE`,
/// Or we can use a simplified rule: `tx.gaslimit = message.gaslimit * 102%`.
function processMessage(
Message calldata _message,
bytes calldata _proof
Expand All @@ -207,7 +218,8 @@ contract Bridge is EssentialContract, IBridge {
sameChain(_message.destChainId)
nonReentrant
{
uint256 gas = gasleft();
GasStats memory stats;
Dismissed Show dismissed Hide dismissed
stats.start = gasleft();

// If the gas limit is set to zero, only the owner can process the message.
if (_message.gasLimit == 0 && msg.sender != _message.destOwner) {
Expand All @@ -233,26 +245,30 @@ contract Bridge is EssentialContract, IBridge {
refundAmount = _message.value;
_updateMessageStatus(msgHash, Status.DONE);
} else {
stats.beforeInvocation = gasleft();
Status status = _invokeMessageCall(
_message, msgHash, _invocationGasLimit(_message, true)
) ? Status.DONE : Status.RETRIABLE;

stats.afterInvocation = gasleft();
_updateMessageStatus(msgHash, status);
}

emit MessageProcessed(_message);

if (_message.fee != 0) {
refundAmount += _message.fee;

if (msg.sender != _message.destOwner && _message.gasLimit != 0) {
unchecked {
uint256 gasUsed = gas - gasleft();
emit GasLog(_message.id, gasUsed, GAS_OVERHEAD, _proof.length, numCacheOps);

uint256 refund = numCacheOps * _GAS_REFUND_PER_CACHE_OPERATION;
gasUsed = (GAS_OVERHEAD + gasUsed).max(refund) - refund;
stats.gasUsedInFeeCalc = stats.start - gasleft();

uint256 gasCharged =
(GAS_OVERHEAD + stats.gasUsedInFeeCalc).max(refund) - refund;

uint256 maxFee = gasUsed * _message.fee / _message.gasLimit;
uint256 baseFee = gasUsed * block.basefee;
uint256 maxFee = gasCharged * _message.fee / _message.gasLimit;
uint256 baseFee = gasCharged * block.basefee;
uint256 fee =
(baseFee >= maxFee ? maxFee : (maxFee + baseFee) >> 1).min(_message.fee);

Expand All @@ -263,6 +279,9 @@ contract Bridge is EssentialContract, IBridge {
}

_message.destOwner.sendEtherAndVerify(refundAmount, _SEND_ETHER_GAS_LIMIT);

stats.end = gasleft();
emit GasLog(msgHash, stats, GAS_OVERHEAD, _proof.length, numCacheOps);
}

/// @inheritdoc IBridge
Expand Down
4 changes: 4 additions & 0 deletions packages/protocol/contracts/bridge/IBridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ interface IBridge {
/// @param message The message.
event MessageSent(bytes32 indexed msgHash, Message message);

/// @notice Emitted when a message is processed.
/// @param message The essage.
event MessageProcessed(Message message);

/// @notice Emitted when the status of a message changes.
/// @param msgHash The hash of the message.
/// @param status The new status of the message.
Expand Down
56 changes: 28 additions & 28 deletions packages/protocol/test/tokenvault/ERC1155Vault.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 2;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 8);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 2);
Expand All @@ -255,11 +255,11 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 2;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(0), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(0), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
vm.expectRevert(BaseNFTVault.VAULT_INVALID_TOKEN.selector);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);
}

function test_1155Vault_sendToken_with_0_tokens_1155() public {
Expand All @@ -276,11 +276,11 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 0;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
vm.expectRevert(BaseNFTVault.VAULT_INVALID_AMOUNT.selector);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);
}

function test_1155Vault_receiveTokens_from_newly_deployed_bridged_contract_on_destination_chain_1155(
Expand All @@ -300,10 +300,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 2;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 8);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 2);
Expand Down Expand Up @@ -355,10 +355,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 2;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 8);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 2);
Expand Down Expand Up @@ -399,10 +399,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 1;

sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 7);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 3);
Expand Down Expand Up @@ -444,7 +444,7 @@ contract ERC1155VaultTest is TaikoTest {
uint256 etherValue = 0.1 ether;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), David, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), David, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: etherValue }(sendOpts);
Expand Down Expand Up @@ -498,11 +498,11 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 2;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);

vm.prank(Alice, Alice);
IBridge.Message memory message = erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
IBridge.Message memory message = erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 8);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 2);
Expand Down Expand Up @@ -533,10 +533,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[1] = 5;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 8);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 2);
Expand Down Expand Up @@ -589,10 +589,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 1;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 1);

Expand Down Expand Up @@ -639,11 +639,11 @@ contract ERC1155VaultTest is TaikoTest {
ERC1155(deployedContract).setApprovalForAll(address(destChainErc1155Vault), true);

sendOpts = BaseNFTVault.BridgeTransferOp(
chainId, address(0), Bob, 500_000, address(deployedContract), 500_000, tokenIds, amounts
chainId, address(0), Bob, 800_000, address(deployedContract), 800_000, tokenIds, amounts
);

vm.prank(Bob, Bob);
destChainErc1155Vault.sendToken{ value: 500_000 }(sendOpts);
destChainErc1155Vault.sendToken{ value: 800_000 }(sendOpts);

vm.chainId(chainId);

Expand Down Expand Up @@ -685,10 +685,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 1;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 1);

Expand Down Expand Up @@ -738,16 +738,16 @@ contract ERC1155VaultTest is TaikoTest {
chainId,
address(0),
Alice,
500_000,
800_000,
address(deployedContract),
500_000,
800_000,
tokenIds,
amounts
);

vm.prank(Alice, Alice);
vm.expectRevert("ERC1155: burn amount exceeds balance");
destChainErc1155Vault.sendToken{ value: 500_000 }(sendOpts);
destChainErc1155Vault.sendToken{ value: 800_000 }(sendOpts);
}

function test_1155Vault_upgrade_bridged_tokens_1155() public {
Expand All @@ -764,10 +764,10 @@ contract ERC1155VaultTest is TaikoTest {
amounts[0] = 2;

BaseNFTVault.BridgeTransferOp memory sendOpts = BaseNFTVault.BridgeTransferOp(
destChainId, address(0), Alice, 500_000, address(ctoken1155), 500_000, tokenIds, amounts
destChainId, address(0), Alice, 800_000, address(ctoken1155), 800_000, tokenIds, amounts
);
vm.prank(Alice, Alice);
erc1155Vault.sendToken{ value: 500_000 }(sendOpts);
erc1155Vault.sendToken{ value: 800_000 }(sendOpts);

assertEq(ctoken1155.balanceOf(Alice, 1), 8);
assertEq(ctoken1155.balanceOf(address(erc1155Vault), 1), 2);
Expand Down
Loading
Loading