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

feat(protocol): restrict max gas paying prover #15200

Merged
merged 5 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 6 additions & 1 deletion packages/protocol/contracts/L1/libs/LibProposing.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ library LibProposing {
// field element has 32 bytes.
uint256 public constant MAX_BYTES_PER_BLOB = 4096 * 32;

// Max gas paying the prover. This should be large enough to prevent the
// worst cases, usually block proposer shall be aware the risks and only
// choose provers that cannot consume too much gas when receiving Ether.
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Easy for provers to give the proposer an address that is an EOA and then deploy a smart contract to that address that consumes a lot of gas!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but provers are most likely entities that win their businesses over time by behaving well, some metrics are important for proposer when to consider which provers to use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't they just pick the prover that offers the lowest fee? Anything that is more trust based seems like a downside for the system to work as well and efficient as possible. Any trust based system would also be more centralizing.

uint256 public constant MAX_GAS_PAYING_PROVER = 200_000;

// Warning: Any events defined here must also be defined in TaikoEvents.sol.
event BlockProposed(
uint256 indexed blockId,
Expand Down Expand Up @@ -334,7 +339,7 @@ library LibProposing {
if (assignment.feeToken == address(0)) {
// Paying Ether
if (msg.value < proverFee) revert L1_ASSIGNMENT_INSUFFICIENT_FEE();
assignment.prover.sendEther(proverFee);
assignment.prover.sendEther(proverFee, MAX_GAS_PAYING_PROVER);
unchecked {
// Return the extra Ether to the proposer
uint256 refund = msg.value - proverFee;
Expand Down
15 changes: 11 additions & 4 deletions packages/protocol/contracts/libs/LibAddress.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ library LibAddress {

error ETH_TRANSFER_FAILED();

/// @dev Sends Ether to the specified address. It is recommended to avoid
/// using `.transfer()` due to potential reentrancy issues.
/// Reference:
/// https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now
/// @dev Sends Ether to the specified address.
/// @param to The recipient address.
/// @param amount The amount of Ether to send in wei.
function sendEther(address to, uint256 amount) internal {
Expand All @@ -42,6 +39,16 @@ library LibAddress {
if (!success) revert ETH_TRANSFER_FAILED();
}

/// @dev Sends Ether to the specified address.
/// @param to The recipient address.
/// @param amount The amount of Ether to send in wei.
/// @param gasLimit The max amount gas to pay for this transaction.
function sendEther(address to, uint256 amount, uint256 gasLimit) internal {
if (to == address(0)) revert ETH_TRANSFER_FAILED();
(bool success,) = payable(to).call{ value: amount, gas: gasLimit }("");
if (!success) revert ETH_TRANSFER_FAILED();
}

function supportsInterface(
address addr,
bytes4 interfaceId
Expand Down