Skip to content

Commit

Permalink
fix: update row estimation with scroll-prover v0.7.2 (#475)
Browse files Browse the repository at this point in the history
* Fix row estimation.

* Update libzkp.

* more

* prepare

* finish

* upgrade

* bump version

* fix tests

* Update to scroll-prover `v0.7.2`.

* fix tests

* Update miner/worker.go

Co-authored-by: Péter Garamvölgyi <[email protected]>

* Update miner/worker.go

Co-authored-by: Péter Garamvölgyi <[email protected]>

* Reset ccc when skips first tx.

* do not unnecessarily skip L1 message

* fix ccc reset and improve code readability

* seal block on circuitcapacitychecker.ErrUnknown

---------

Co-authored-by: HAOYUatHZ <[email protected]>
Co-authored-by: Péter Garamvölgyi <[email protected]>
  • Loading branch information
3 people committed Aug 25, 2023
1 parent 44e936c commit 55f04ca
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 80 deletions.
77 changes: 38 additions & 39 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1070,42 +1070,41 @@ loop:
log.Trace("Skipping unsupported transaction type", "sender", from, "type", tx.Type())
txs.Pop()

// Circuit capacity check
case errors.Is(err, circuitcapacitychecker.ErrBlockRowConsumptionOverflow):
// Circuit capacity check: circuit capacity limit reached in a block,
// don't pop or shift, just quit the loop immediately;
// though it might still be possible to add some "smaller" txs,
// but it's a trade-off between tracing overhead & block usage rate
log.Trace("Circuit capacity limit reached in a block", "acc_rows", w.current.accRows, "tx", tx.Hash().String())
circuitCapacityReached = true
break loop

case (errors.Is(err, circuitcapacitychecker.ErrTxRowConsumptionOverflow) && tx.IsL1MessageTx()):
// Circuit capacity check: L1MessageTx row consumption too high, shift to the next from the account,
// because we shouldn't skip the entire txs from the same account.
// This is also useful for skipping "problematic" L1MessageTxs.
queueIndex := tx.AsL1MessageTx().QueueIndex
log.Trace("Circuit capacity limit reached for a single tx", "tx", tx.Hash().String(), "queueIndex", queueIndex)
log.Info("Skipping L1 message", "queueIndex", queueIndex, "tx", tx.Hash().String(), "block", w.current.header.Number, "reason", "row consumption overflow")
w.current.nextL1MsgIndex = queueIndex + 1
rawdb.WriteSkippedTransaction(w.eth.ChainDb(), tx, "row consumption overflow", w.current.header.Number.Uint64(), nil)

// after `ErrTxRowConsumptionOverflow`, ccc might not revert updates
// associated with this transaction so we cannot pack more transactions.
// TODO: fix this in ccc and change these lines back to `txs.Shift()`
circuitCapacityReached = true
break loop
if w.current.tcount >= 1 {
// 1. Circuit capacity limit reached in a block, and it's not the first tx:
// don't pop or shift, just quit the loop immediately;
// though it might still be possible to add some "smaller" txs,
// but it's a trade-off between tracing overhead & block usage rate
log.Trace("Circuit capacity limit reached in a block", "acc_rows", w.current.accRows, "tx", tx.Hash().String())
circuitCapacityReached = true
break loop
} else {
// 2. Circuit capacity limit reached in a block, and it's the first tx: skip the tx
log.Trace("Circuit capacity limit reached for a single tx", "tx", tx.Hash().String())

if tx.IsL1MessageTx() {
// Skip L1 message transaction,
// shift to the next from the account because we shouldn't skip the entire txs from the same account
txs.Shift()

queueIndex := tx.AsL1MessageTx().QueueIndex
log.Info("Skipping L1 message", "queueIndex", queueIndex, "tx", tx.Hash().String(), "block", w.current.header.Number, "reason", "row consumption overflow")
w.current.nextL1MsgIndex = queueIndex + 1
} else {
// Skip L2 transaction and all other transactions from the same sender account
txs.Pop()
}

case (errors.Is(err, circuitcapacitychecker.ErrTxRowConsumptionOverflow) && !tx.IsL1MessageTx()):
// Circuit capacity check: L2MessageTx row consumption too high, skip the account.
// This is also useful for skipping "problematic" L2MessageTxs.
log.Trace("Circuit capacity limit reached for a single tx", "tx", tx.Hash().String())
rawdb.WriteSkippedTransaction(w.eth.ChainDb(), tx, "row consumption overflow", w.current.header.Number.Uint64(), nil)
// Reset ccc so that we can process other transactions for this block
w.circuitCapacityChecker.Reset()
log.Trace("Worker reset ccc", "id", w.circuitCapacityChecker.ID)
circuitCapacityReached = false

// after `ErrTxRowConsumptionOverflow`, ccc might not revert updates
// associated with this transaction so we cannot pack more transactions.
// TODO: fix this in ccc and change these lines back to `txs.Pop()`
circuitCapacityReached = true
break loop
// Store skipped transaction in local db
rawdb.WriteSkippedTransaction(w.eth.ChainDb(), tx, "row consumption overflow", w.current.header.Number.Uint64(), nil)
}

case (errors.Is(err, circuitcapacitychecker.ErrUnknown) && tx.IsL1MessageTx()):
// Circuit capacity check: unknown circuit capacity checker error for L1MessageTx,
Expand All @@ -1117,19 +1116,19 @@ loop:
// TODO: propagate more info about the error from CCC
rawdb.WriteSkippedTransaction(w.eth.ChainDb(), tx, "unknown row consumption error", w.current.header.Number.Uint64(), nil)

// after `ErrUnknown`, ccc might not revert updates associated
// with this transaction so we cannot pack more transactions.
// TODO: fix this in ccc and change these lines back to `txs.Shift()`
// Normally we would do `txs.Shift()` here.
// However, after `ErrUnknown`, ccc might remain in an
// inconsistent state, so we cannot pack more transactions.
circuitCapacityReached = true
break loop

case (errors.Is(err, circuitcapacitychecker.ErrUnknown) && !tx.IsL1MessageTx()):
// Circuit capacity check: unknown circuit capacity checker error for L2MessageTx, skip the account
log.Trace("Unknown circuit capacity checker error for L2MessageTx", "tx", tx.Hash().String())

// after `ErrUnknown`, ccc might not revert updates associated
// with this transaction so we cannot pack more transactions.
// TODO: fix this in ccc and change these lines back to `txs.Pop()`
// Normally we would do `txs.Pop()` here.
// However, after `ErrUnknown`, ccc might remain in an
// inconsistent state, so we cannot pack more transactions.
circuitCapacityReached = true
break loop

Expand Down
22 changes: 19 additions & 3 deletions miner/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1010,10 +1010,10 @@ func TestOversizedTxThenNormal(t *testing.T) {
switch blockNum {
case 0:
// schedule to skip 2nd call to ccc
w.getCCC().ScheduleError(2, circuitcapacitychecker.ErrTxRowConsumptionOverflow)
w.getCCC().ScheduleError(2, circuitcapacitychecker.ErrBlockRowConsumptionOverflow)
return false
case 1:
// include #0, skip #1, then terminate
// include #0, fail on #1, then seal the block
assert.Equal(1, len(block.Transactions()))

assert.True(block.Transactions()[0].IsL1MessageTx())
Expand All @@ -1022,7 +1022,23 @@ func TestOversizedTxThenNormal(t *testing.T) {
// db is updated correctly
queueIndex := rawdb.ReadFirstQueueIndexNotInL2Block(db, block.Hash())
assert.NotNil(queueIndex)
assert.Equal(uint64(2), *queueIndex)
assert.Equal(uint64(1), *queueIndex)

// schedule to skip next call to ccc
w.getCCC().ScheduleError(1, circuitcapacitychecker.ErrBlockRowConsumptionOverflow)

return false
case 2:
// skip #1, include #2, then seal the block
assert.Equal(1, len(block.Transactions()))

assert.True(block.Transactions()[0].IsL1MessageTx())
assert.Equal(uint64(2), block.Transactions()[0].AsL1MessageTx().QueueIndex)

// db is updated correctly
queueIndex := rawdb.ReadFirstQueueIndexNotInL2Block(db, block.Hash())
assert.NotNil(queueIndex)
assert.Equal(uint64(3), *queueIndex)

return true
default:
Expand Down
2 changes: 1 addition & 1 deletion params/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
const (
VersionMajor = 4 // Major version component of the current release
VersionMinor = 3 // Minor version component of the current release
VersionPatch = 47 // Patch version component of the current release
VersionPatch = 48 // Patch version component of the current release
VersionMeta = "sepolia" // Version metadata to append to the version string
)

Expand Down
8 changes: 2 additions & 6 deletions rollup/circuitcapacitychecker/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,13 @@ func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*
log.Error("fail to apply_tx in CircuitCapacityChecker", "id", ccc.ID, "TxHash", traces.Transactions[0].TxHash, "err", result.Error)
return nil, ErrUnknown
}
if result.TxRowUsage == nil || result.AccRowUsage == nil {
if result.AccRowUsage == nil {
log.Error("fail to apply_tx in CircuitCapacityChecker",
"id", ccc.ID, "TxHash", traces.Transactions[0].TxHash,
"result.TxRowUsage==nil", result.TxRowUsage == nil,
"result.AccRowUsage == nil", result.AccRowUsage == nil,
"err", "TxRowUsage or AccRowUsage is empty unexpectedly")
"err", "AccRowUsage is empty unexpectedly")
return nil, ErrUnknown
}
if !result.TxRowUsage.IsOk {
return nil, ErrTxRowConsumptionOverflow
}
if !result.AccRowUsage.IsOk {
return nil, ErrBlockRowConsumptionOverflow
}
Expand Down
28 changes: 14 additions & 14 deletions rollup/circuitcapacitychecker/libzkp/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions rollup/circuitcapacitychecker/libzkp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ maingate = { git = "https://github.com/scroll-tech/halo2wrong", branch = "halo2-
halo2curves = { git = "https://github.com/scroll-tech/halo2curves.git", branch = "0.3.1-derive-serde" }

[dependencies]
prover = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.6.4" }
types = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.6.4" }
prover = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.7.2" }
types = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.7.2" }

log = "0.4"
env_logger = "0.9.0"
Expand Down
27 changes: 14 additions & 13 deletions rollup/circuitcapacitychecker/libzkp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ pub mod checker {
#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct RowUsageResult {
pub acc_row_usage: Option<RowUsage>,
pub tx_row_usage: Option<RowUsage>,
pub error: Option<String>,
}

Expand Down Expand Up @@ -59,7 +58,11 @@ pub mod checker {
#[no_mangle]
pub unsafe extern "C" fn apply_tx(id: u64, tx_traces: *const c_char) -> *const c_char {
let result = panic::catch_unwind(|| {
log::debug!("ccc apply_tx raw input, id: {:?}, tx_traces: {:?}", id, c_char_to_str(tx_traces));
log::debug!(
"ccc apply_tx raw input, id: {:?}, tx_traces: {:?}",
id,
c_char_to_str(tx_traces)
);
let tx_traces_vec = c_char_to_vec(tx_traces);
let traces = serde_json::from_slice::<BlockTrace>(&tx_traces_vec)
.unwrap_or_else(|_| panic!("id: {id:?}, fail to deserialize tx_traces"));
Expand Down Expand Up @@ -88,22 +91,19 @@ pub mod checker {
})
});
let r = match result {
Ok((acc_row_usage, tx_row_usage)) => {
Ok(acc_row_usage) => {
log::debug!(
"id: {:?}, acc_row_usage: {:?}, tx_row_usage: {:?}",
"id: {:?}, acc_row_usage: {:?}",
id,
acc_row_usage.row_number,
tx_row_usage.row_number
);
RowUsageResult {
acc_row_usage: Some(acc_row_usage),
tx_row_usage: Some(tx_row_usage),
error: None,
}
}
Err(e) => RowUsageResult {
acc_row_usage: None,
tx_row_usage: None,
error: Some(format!("{e:?}")),
},
};
Expand All @@ -114,7 +114,11 @@ pub mod checker {
#[no_mangle]
pub unsafe extern "C" fn apply_block(id: u64, block_trace: *const c_char) -> *const c_char {
let result = panic::catch_unwind(|| {
log::debug!("ccc apply_block raw input, id: {:?}, block_trace: {:?}", id, c_char_to_str(block_trace));
log::debug!(
"ccc apply_block raw input, id: {:?}, block_trace: {:?}",
id,
c_char_to_str(block_trace)
);
let block_trace = c_char_to_vec(block_trace);
let traces = serde_json::from_slice::<BlockTrace>(&block_trace)
.unwrap_or_else(|_| panic!("id: {id:?}, fail to deserialize block_trace"));
Expand All @@ -136,22 +140,19 @@ pub mod checker {
})
});
let r = match result {
Ok((acc_row_usage, tx_row_usage)) => {
Ok(acc_row_usage) => {
log::debug!(
"id: {:?}, acc_row_usage: {:?}, tx_row_usage: {:?}",
"id: {:?}, acc_row_usage: {:?}",
id,
acc_row_usage.row_number,
tx_row_usage.row_number
);
RowUsageResult {
acc_row_usage: Some(acc_row_usage),
tx_row_usage: Some(tx_row_usage),
error: None,
}
}
Err(e) => RowUsageResult {
acc_row_usage: None,
tx_row_usage: None,
error: Some(format!("{e:?}")),
},
};
Expand Down
2 changes: 0 additions & 2 deletions rollup/circuitcapacitychecker/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ import (

var (
ErrUnknown = errors.New("unknown circuit capacity checker error")
ErrTxRowConsumptionOverflow = errors.New("tx row consumption overflow")
ErrBlockRowConsumptionOverflow = errors.New("block row consumption overflow")
)

type WrappedRowUsage struct {
AccRowUsage *types.RowUsage `json:"acc_row_usage,omitempty"`
TxRowUsage *types.RowUsage `json:"tx_row_usage,omitempty"`
Error string `json:"error,omitempty"`
}

0 comments on commit 55f04ca

Please sign in to comment.