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): remove gas limit check #14087

Closed
wants to merge 21 commits into from
Closed

feat(protocol): remove gas limit check #14087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
98f9498
Remove gasLimit
dong77 Jul 2, 2023
71f5b36
more
dong77 Jul 2, 2023
92d787a
Update LibVerifying.sol
dong77 Jul 2, 2023
43fcd7a
block.prevrandao
dantaik Jul 3, 2023
fac5ca3
Merge branch 'prevrandao' into remove_gas_limit_check
dantaik Jul 3, 2023
6c12ba4
merge main
dong77 Jul 27, 2023
5afb36e
merge main
dong77 Jul 27, 2023
ba51f1e
Update lefthook.yml
dong77 Jul 27, 2023
57553a3
Update eventindexer.yml
dantaik Jul 27, 2023
3e5a7d4
Update .gitignore
dantaik Jul 27, 2023
0a50f8e
Add auto-generated contract documentation
dantaik Jul 27, 2023
4f29af6
update doc
dong77 Jul 27, 2023
883c2fb
update doc
dong77 Jul 28, 2023
a621929
Update how_taiko_proves_blocks.md
dong77 Jul 28, 2023
7cd9859
Update how_taiko_proves_blocks.md
dong77 Jul 28, 2023
30d98b1
add back maxGasLimit
dong77 Jul 28, 2023
8707d4e
Add auto-generated contract documentation
dantaik Jul 28, 2023
6ea1180
Merge branch 'alpha-4' into remove_gas_limit_check
dantaik Jul 28, 2023
37b29bd
Merge branch 'alpha-4' into remove_gas_limit_check
dantaik Jul 29, 2023
e6eabc9
fix feedback
dantaik Jul 29, 2023
ad670e6
Update LibVerifying.sol
dantaik Jul 31, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/eventindexer.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,4 @@ jobs:
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
build-args: |
PACKAGE=eventindexer
PACKAGE=eventindexer
12 changes: 6 additions & 6 deletions lefthook.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
pre-commit:
parallel: true
commands:
# bridge-ui:
# glob: "packages/bridge-ui/**.{js,svelte,ts}"
# run: pnpm -F bridge-ui lint:fix && git add {staged_files}
# bridge-ui-v2:
# glob: "packages/bridge-ui-v2/**.{js,ts,css,svelte}"
# run: pnpm -F bridge-ui-v2 svelte:check && pnpm -F bridge-ui-v2 lint:fix && git add {staged_files}
bridge-ui:
glob: "packages/bridge-ui/**.{js,svelte,ts}"
run: pnpm -F bridge-ui lint:fix && git add {staged_files}
bridge-ui-v2:
glob: "packages/bridge-ui-v2/**.{js,ts,css,svelte}"
run: pnpm -F bridge-ui-v2 svelte:check && pnpm -F bridge-ui-v2 lint:fix && git add {staged_files}
protocol_sol:
glob: "packages/protocol/**.{sol}"
run: pnpm -F protocol lint:sol && git add {staged_files}
Expand Down
1 change: 0 additions & 1 deletion packages/pos-dashboard/temp.txt
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion packages/protocol/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ broadcast
.vscode

#coverage
lcov.info
lcov.info
2 changes: 1 addition & 1 deletion packages/protocol/contracts/L1/TaikoConfig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ library TaikoConfig {
blockMaxVerificationsPerTx: 10,
// Set it to 6M, since its the upper limit of the Alpha-2
// testnet's circuits.
blockMaxGasLimit: 6_000_000,
blockAndTxMaxGasUsed: 6_000_000,
blockFeeBaseGas: 20_000,
// Set it to 79 (+1 TaikoL2.anchor transaction = 80),
// and 80 is the upper limit of the Alpha-2 testnet's circuits.
Expand Down
10 changes: 4 additions & 6 deletions packages/protocol/contracts/L1/TaikoData.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ library TaikoData {
// This number is calculated from blockMaxProposals to make
// the 'the maximum value of the multiplier' close to 20.0
uint256 blockMaxVerificationsPerTx;
uint32 blockMaxGasLimit;
// The max gas used per block, including anchor.
uint32 blockAndTxMaxGasUsed;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to double check if we will use this value to reject transactions with tx.limit > blockAndTxMaxGasUsed because then the name is a bit confusing (gasLimit vs gasUsed). I'm not sure if Ethereum itself already does something like this e.g. if a tx has a gas limit of 100M gas but the block gas limit is 10M gas, is it rejected or can it still be included in a block if the tx ends up only using < 10M gas?

Copy link
Member

@davidtaikocha davidtaikocha Jul 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if Ethereum itself already does something like this e.g. if a tx has a gas limit of 100M gas but the block gas limit is 10M gas, is it rejected or can it still be included in a block if the tx ends up only using < 10M gas?

image

No all transactions accumulated gasLimit can not exceed block.gasLimit, so in go-ethereum, if its building a block, that transaction will just be rejected; if it received a block like this from the network, the whole block will be treat as invalid. So i think, if we just use blockAndTxMaxGasUsed as block.gasLimit, it will break the ethereum protocol, maybe use uint256.max or just set a txMaxGasLimit then use the all txs accumulated gasLimit as the block.gasLimit will be more ethereum competitive?

Copy link
Contributor

@Brechtpd Brechtpd Jul 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

So Ethereum has a pretty similar problem we currently have? As in only a single tx with a tx.gasLimit close to the block gas limit can be included? The only difference with our previous approach is that transactions with a smaller tx.gasLimit cannot be included anymore while in Ethereum blocks that would still be possible.

I think blockAndTxMaxGasUsed should still be okay no? So there is a block gas limit, and each tx.gasLimit needs to be <= block.gasLimit. But to be 100% compatible with Ethereum we would also have to add an additional check that l(Br)_u + tx.gasLimit <= block.gasLimit as well. That seems like a pretty easy check to add.

Copy link
Contributor

Choose a reason for hiding this comment

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

set a txMaxGasLimit then use the all txs accumulated gasLimit as the block.gasLimit will be more ethereum competitive?

I think this would be a pretty big change (similar to the previous approach) where the the tx.gasLimit is a pretty important value. In Ethereum it's a mostly unimportant value where you can set it to a high value to make sure it will be successful with no real impact (except I guess for the rule above, but for any real use case should still be much smaller than block.gasLimit).

uint32 blockFeeBaseGas;
uint64 blockMaxTransactions;
uint64 blockMaxTxListBytes;
uint32 blockMaxTransactions;
uint32 blockMaxTxListBytes;
uint256 blockTxListExpiry;
// Group 3: proof related configs
uint256 proofRegularCooldown;
Expand Down Expand Up @@ -56,7 +57,6 @@ library TaikoData {
struct BlockMetadataInput {
bytes32 txListHash;
address beneficiary;
uint32 gasLimit;
davidtaikocha marked this conversation as resolved.
Show resolved Hide resolved
uint24 txListByteStart; // byte-wise start index (inclusive)
uint24 txListByteEnd; // byte-wise end index (exclusive)
bool cacheTxListInfo;
Expand All @@ -72,7 +72,6 @@ library TaikoData {
bytes32 txListHash;
uint24 txListByteStart;
uint24 txListByteEnd;
uint32 gasLimit;
address beneficiary;
address treasury;
TaikoData.EthDeposit[] depositsProcessed;
Expand Down Expand Up @@ -109,7 +108,6 @@ library TaikoData {
bytes32 metaHash;
// slot 3: (13 bytes available)
uint64 blockId;
uint32 gasLimit;
uint24 nextForkChoiceId;
uint24 verifiedForkChoiceId;
bool proverReleased;
Expand Down
10 changes: 2 additions & 8 deletions packages/protocol/contracts/L1/TaikoL1.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,8 @@ contract TaikoL1 is
});
}

function getBlockFee(uint32 gasLimit) public view returns (uint64) {
return LibUtils.getBlockFee({
state: state,
config: getConfig(),
gasAmount: gasLimit
});
function getBlockFee() public view returns (uint64) {
Copy link
Member

Choose a reason for hiding this comment

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

Shall we also change the calculation of L2 EIP-1559? since its "purchasing" the block.gaslimit.

Copy link
Contributor

Choose a reason for hiding this comment

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

If block.gaslimit is set to blockMaxGasUsed I think it's fine.

return LibUtils.getBlockFee({ state: state, config: getConfig() });
}

function getTaikoTokenBalance(address addr) public view returns (uint256) {
Expand All @@ -193,7 +189,6 @@ contract TaikoL1 is
view
returns (
bytes32 _metaHash,
uint32 _gasLimit,
uint24 _nextForkChoiceId,
uint24 _verifiedForkChoiceId,
bool _proverReleased,
Expand All @@ -211,7 +206,6 @@ contract TaikoL1 is
blockId: blockId
});
_metaHash = blk.metaHash;
_gasLimit = blk.gasLimit;
_nextForkChoiceId = blk.nextForkChoiceId;
_verifiedForkChoiceId = blk.verifiedForkChoiceId;
_proverReleased = blk.proverReleased;
Expand Down
12 changes: 2 additions & 10 deletions packages/protocol/contracts/L1/libs/LibProposing.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ library LibProposing {
meta.txListHash = input.txListHash;
meta.txListByteStart = input.txListByteStart;
meta.txListByteEnd = input.txListByteEnd;
meta.gasLimit = input.gasLimit;
meta.beneficiary = input.beneficiary;
meta.treasury = resolver.resolve(config.chainId, "treasury", false);
meta.depositsProcessed =
Expand All @@ -115,7 +114,6 @@ library LibProposing {

blk.metaHash = LibUtils.hashMetadata(meta);
blk.blockId = state.numBlocks;
blk.gasLimit = meta.gasLimit;
blk.nextForkChoiceId = 1;
blk.verifiedForkChoiceId = 0;
blk.proverReleased = false;
Expand All @@ -140,7 +138,7 @@ library LibProposing {
);
}

uint64 blockFee = LibUtils.getBlockFee(state, config, meta.gasLimit);
uint64 blockFee = LibUtils.getBlockFee(state, config);

if (state.taikoTokenBalances[msg.sender] < blockFee) {
revert L1_INSUFFICIENT_TOKEN();
Expand Down Expand Up @@ -183,13 +181,7 @@ library LibProposing {
view
returns (bool cacheTxListInfo)
{
if (
input.beneficiary == address(0)
//
|| input.gasLimit == 0
//
|| input.gasLimit > config.blockMaxGasLimit
) revert L1_INVALID_METADATA();
if (input.beneficiary == address(0)) revert L1_INVALID_METADATA();

if (
state.numBlocks
Expand Down
8 changes: 5 additions & 3 deletions packages/protocol/contracts/L1/libs/LibProving.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ library LibProving {
|| evidence.signalRoot == 0
//
|| evidence.gasUsed == 0
//
|| evidence.gasUsed > config.blockAndTxMaxGasUsed
) revert L1_INVALID_EVIDENCE();

if (blockId <= state.lastVerifiedBlockId || blockId >= state.numBlocks)
Expand Down Expand Up @@ -174,9 +176,9 @@ library LibProving {
| (uint256(evidence.gasUsed) << 32);

// Also hash configs that will be used by circuits
inputs[9] = uint256(config.blockMaxGasLimit) << 192
| uint256(config.blockMaxTransactions) << 128
| uint256(config.blockMaxTxListBytes) << 64;
inputs[9] = uint256(config.blockAndTxMaxGasUsed) << 224 // 32 bits
| uint256(config.blockMaxTransactions) << 192 // 32 bits
| uint256(config.blockMaxTxListBytes) << 160; // 32 bits

assembly {
instance := keccak256(inputs, mul(32, 10))
Expand Down
12 changes: 8 additions & 4 deletions packages/protocol/contracts/L1/libs/LibUtils.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,21 @@ library LibUtils {
});
}

// If auction is tied to gas, we should charge users based on gas as well. At
// this point gasUsed (in proposeBlock()) is always gasLimit, so use it and
// in case of differences refund after verification
function getBlockFee(
TaikoData.State storage state,
TaikoData.Config memory config,
uint32 gasAmount
TaikoData.Config memory config
)
internal
view
returns (uint64)
{
// The diff between gasLimit and gasUsed will be redistributed back to
// the balance of proposer
return state.feePerGas
* (gasAmount + LibL2Consts.ANCHOR_GAS_COST + config.blockFeeBaseGas);
* (config.blockAndTxMaxGasUsed + config.blockFeeBaseGas);
}

function movingAverage(
Expand Down Expand Up @@ -120,7 +124,7 @@ library LibUtils {

inputs[5] = (uint256(meta.txListByteStart) << 232)
| (uint256(meta.txListByteEnd) << 208) //
| (uint256(meta.gasLimit) << 176)
// | (uint256(meta.gasLimit) << 176) // Room for a uint32
| (uint256(uint160(meta.beneficiary)) << 16);

inputs[6] = (uint256(uint160(meta.treasury)) << 96);
Expand Down
15 changes: 6 additions & 9 deletions packages/protocol/contracts/L1/libs/LibVerifying.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ library LibVerifying {
config.chainId <= 1 //
|| config.blockMaxProposals == 1
|| config.blockRingBufferSize <= config.blockMaxProposals + 1
|| config.blockMaxGasLimit == 0 || config.blockMaxTransactions == 0
|| config.blockAndTxMaxGasUsed <= LibL2Consts.ANCHOR_GAS_COST
|| config.blockMaxTransactions == 0
|| uint256(config.blockMaxTransactions)
* config.blockMaxTransactions >= type(uint32).max
|| config.blockMaxTxListBytes == 0
|| config.blockTxListExpiry > 30 * 24 hours
|| config.blockMaxTxListBytes > 128 * 1024 //blob up to 128K
Expand Down Expand Up @@ -193,12 +196,6 @@ library LibVerifying {
)
private
{
// the actually mined L2 block's gasLimit is blk.gasLimit +
// LibL2Consts.ANCHOR_GAS_COST, so fc.gasUsed may greater than
// blk.gasLimit here.
uint32 _gasLimit = blk.gasLimit + LibL2Consts.ANCHOR_GAS_COST;
assert(fc.gasUsed <= _gasLimit);

IProverPool proverPool =
IProverPool(resolver.resolve("prover_pool", false));

Expand Down Expand Up @@ -262,7 +259,7 @@ library LibVerifying {

// refund the proposer
state.taikoTokenBalances[blk.proposer] +=
(_gasLimit - fc.gasUsed) * blk.feePerGas;
(config.blockAndTxMaxGasUsed - fc.gasUsed) * blk.feePerGas;

// Reward the prover
state.taikoTokenBalances[fc.prover] += proofReward;
Expand All @@ -271,7 +268,7 @@ library LibVerifying {
blockId: blk.blockId,
blockHash: fc.blockHash,
prover: fc.prover,
blockFee: LibUtils.getBlockFee(state, config, fc.gasUsed),
blockFee: LibUtils.getBlockFee(state, config),
proofReward: proofReward
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/bridge/IBridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ interface IBridge {
uint256 destChainId;
// Owner address of the bridged asset.
address owner;
// Destination owner address.
// Target invocation address.
address to;
// Alternate address to send any refund. If blank, defaults to owner.
address refundAddress;
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/test/L1/TestTaikoL1.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract TestTaikoL1 is TaikoL1 {
// This number is calculated from blockMaxProposals to make
// the 'the maximum value of the multiplier' close to 20.0
config.blockMaxVerificationsPerTx = 0;
config.blockMaxGasLimit = 30_000_000;
config.blockAndTxMaxGasUsed = 30_000_000;
config.blockMaxTransactions = 20;
config.blockMaxTxListBytes = 120_000;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/test/L1/TestTaikoL1EnableTokenomics.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract TestTaikoL1EnableTokenomics is TaikoL1 {
// the 'the maximum value of the multiplier' close to 20.0
config.blockMaxVerificationsPerTx = 0; // dont verify blocks
// automatically
config.blockMaxGasLimit = 30_000_000;
config.blockAndTxMaxGasUsed = 30_000_000;
config.blockMaxTransactions = 20;
config.blockMaxTxListBytes = 120_000;
}
Expand Down
12 changes: 9 additions & 3 deletions packages/protocol/docs/how_taiko_proves_blocks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ A valid `txList` (until [issue #13724](https://github.com/taikoxyz/taiko-mono/is
- Has a byte-size smaller than the protocol constant _`blockMaxTxListBytes`_ (also enforced in contracts);
- Can be RLP-decoded into a list of transactions without trailing space;
- Contains no more transactions (valid and invalid) than the protocol constant _`blockMaxTransactions`_;
- Has a total gas limit for all valid transactions not exceeding the protocol constant _`blockMaxGasLimit`_;
- Has a total gas limit for all valid transactions not exceeding the protocol constant _`blockAndTxMaxGasUsed * blockMaxTransactions`_;

ZKP must prove whether the `txList` is valid or invalid. For an invalid `txList`, the corresponding L2 block will only have an anchor transaction.

Expand Down Expand Up @@ -204,9 +204,16 @@ classDef otherCircuits stroke-width:4px,stroke:#6ECEB0,fill:#6ECEB0,color:#FFF;
classDef constant stroke-width:4px,stroke:#323745,fill:#323745,color:#FFF;
classDef group stroke-width:2px,stroke:#EA27C2,fill:#FFD2F630;

c_block_gas_limit_max(blockAndTxMaxGasUsed * blockMaxTransactions):::constant;
c_block_gas_used_max(blockAndTxMaxGasUsed):::constant;

m_id --- h_height --- v_block_number;
m_h1_height --- a_l1_height;
m_gas_limit --- h_gas_limit --- v_block_gaslimit;
h_gas_limit --- v_block_gaslimit

c_block_gas_limit_max -->|>=| h_gas_limit;
h_gas_used ---|<=|c_block_gas_used_max;

m_timestamp --- h_timestamp --- v_block_timestamp;
m_txlist_first ---|<=| m_txlist_last --- |<= len| tx_list;
m_h1_hash --- a_l1_hash;
Expand Down Expand Up @@ -250,7 +257,6 @@ BlockEvidence ~~~ L1Storage;

subgraph BlockMetadata[Block Metadata]
m_id(id)
m_gas_limit(gasLimit)
m_timestamp(timestamp)
m_h1_height(h1Height)
m_h1_hash(h1Hash)
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"hardhat-docgen": "^1.3.0",
"hardhat-gas-reporter": "^1.0.9",
"hardhat-preprocessor": "^0.1.5",
"lefthook": "^1.4.7",
"merkle-patricia-tree": "^4.2.4",
"prettier": "^2.8.8",
"rlp": "^3.0.0",
Expand Down
2 changes: 0 additions & 2 deletions packages/protocol/test/GasComparison.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ contract FooBar {
txListHash: bytes32(uint256(1)),
txListByteStart: 0,
txListByteEnd: 1000,
gasLimit: 1,
mixHash: bytes32(uint256(1)),
timestamp: 1,
depositsProcessed: new TaikoData.EthDeposit[](0)
Expand All @@ -102,7 +101,6 @@ contract FooBar {
txListHash: bytes32(uint256(1)),
txListByteStart: 0,
txListByteEnd: 1000,
gasLimit: 1,
mixHash: bytes32(uint256(1)),
timestamp: 1,
depositsProcessed: new TaikoData.EthDeposit[](0)
Expand Down
Loading