Skip to content

Commit

Permalink
fix (libzkp): check tx num when CCC ErrUnknown error occurred (#505)
Browse files Browse the repository at this point in the history
* Check tx num for CCC ErrUnknown error.

* Update worker.go

* Small fix.

* Update

* Fix log.

* Update version.

* Update miner/worker.go

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* Update according to review.

* Fix to return 0 for uint64.

* Add `tcount` to mock CCC, and function `SetTxNum`.

* refactor

* fix

* add comments

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
Co-authored-by: HAOYUatHZ <haoyu@protonmail.com>
  • Loading branch information
4 people committed Sep 8, 2023
1 parent dade96d commit 7d79f27
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 1 deletion.
13 changes: 13 additions & 0 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,7 @@ loop:
// However, after `ErrUnknown`, ccc might remain in an
// inconsistent state, so we cannot pack more transactions.
circuitCapacityReached = true
w.checkCurrentTxNumWithCCC(w.current.tcount)
break loop

case (errors.Is(err, circuitcapacitychecker.ErrUnknown) && !tx.IsL1MessageTx()):
Expand All @@ -1164,6 +1165,7 @@ loop:
// inconsistent state, so we cannot pack more transactions.
w.eth.TxPool().RemoveTx(tx.Hash(), true)
circuitCapacityReached = true
w.checkCurrentTxNumWithCCC(w.current.tcount)
break loop

default:
Expand Down Expand Up @@ -1208,6 +1210,17 @@ loop:
return false, circuitCapacityReached
}

func (w *worker) checkCurrentTxNumWithCCC(expected int) {
match, got, err := w.circuitCapacityChecker.CheckTxNum(expected)
if err != nil {
log.Error("failed to CheckTxNum in ccc", "err", err)
return
}
if !match {
log.Error("tx count in miner is different with CCC", "w.current.tcount", w.current.tcount, "got", got)
}
}

func (w *worker) collectPendingL1Messages(startIndex uint64) []types.L1MessageTx {
maxCount := w.chainConfig.Scroll.L1Config.NumL1MessagesPerBlock
return rawdb.ReadL1MessagesFrom(w.eth.ChainDb(), startIndex, maxCount)
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 = 4 // Minor version component of the current release
VersionPatch = 2 // Patch version component of the current release
VersionPatch = 3 // Patch version component of the current release
VersionMeta = "sepolia" // Version metadata to append to the version string
)

Expand Down
29 changes: 29 additions & 0 deletions rollup/circuitcapacitychecker/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type CircuitCapacityChecker struct {
ID uint64
}

// NewCircuitCapacityChecker creates a new CircuitCapacityChecker
func NewCircuitCapacityChecker() *CircuitCapacityChecker {
creationMu.Lock()
defer creationMu.Unlock()
Expand All @@ -39,13 +40,15 @@ func NewCircuitCapacityChecker() *CircuitCapacityChecker {
return &CircuitCapacityChecker{ID: uint64(id)}
}

// Reset resets a CircuitCapacityChecker
func (ccc *CircuitCapacityChecker) Reset() {
ccc.Lock()
defer ccc.Unlock()

C.reset_circuit_capacity_checker(C.uint64_t(ccc.ID))
}

// ApplyTransaction appends a tx's wrapped BlockTrace into the ccc, and return the accumulated RowConsumption
func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*types.RowConsumption, error) {
ccc.Lock()
defer ccc.Unlock()
Expand Down Expand Up @@ -100,6 +103,7 @@ func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*
return (*types.RowConsumption)(&result.AccRowUsage.RowUsageDetails), nil
}

// ApplyBlock gets a block's RowConsumption
func (ccc *CircuitCapacityChecker) ApplyBlock(traces *types.BlockTrace) (*types.RowConsumption, error) {
ccc.Lock()
defer ccc.Unlock()
Expand Down Expand Up @@ -141,3 +145,28 @@ func (ccc *CircuitCapacityChecker) ApplyBlock(traces *types.BlockTrace) (*types.
}
return (*types.RowConsumption)(&result.AccRowUsage.RowUsageDetails), nil
}

// CheckTxNum compares whether the tx_count in ccc match the expected
func (ccc *CircuitCapacityChecker) CheckTxNum(expected int) (bool, unit64, error) {

Check failure on line 150 in rollup/circuitcapacitychecker/impl.go

View workflow job for this annotation

GitHub Actions / build-geth

undefined: unit64
ccc.Lock()
defer ccc.Unlock()

log.Debug("ccc get_tx_num start", "id", ccc.ID)
rawResult := C.get_tx_num(C.uint64_t(ccc.ID))
defer func() {
C.free(unsafe.Pointer(rawResult))
}()
log.Debug("ccc get_tx_num end", "id", ccc.ID)

result := &WrappedTxNum{}
if err = json.Unmarshal([]byte(C.GoString(rawResult)), result); err != nil {

Check failure on line 162 in rollup/circuitcapacitychecker/impl.go

View workflow job for this annotation

GitHub Actions / build-geth

undefined: err

Check failure on line 162 in rollup/circuitcapacitychecker/impl.go

View workflow job for this annotation

GitHub Actions / build-geth

undefined: err
log.Error("fail to json unmarshal get_tx_num result", "id", ccc.ID, "err", err)

Check failure on line 163 in rollup/circuitcapacitychecker/impl.go

View workflow job for this annotation

GitHub Actions / build-geth

undefined: err
return false, 0, ErrUnknown
}
if result.Error != "" {
log.Error("fail to get_tx_num in CircuitCapacityChecker", "id", ccc.ID, "err", result.Error)
return false, 0, ErrUnknown
}

return result.TxNum == unit64(expected), result.TxNum, nil

Check failure on line 171 in rollup/circuitcapacitychecker/impl.go

View workflow job for this annotation

GitHub Actions / build-geth

undefined: unit64
}
1 change: 1 addition & 0 deletions rollup/circuitcapacitychecker/libzkp/libzkp.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ uint64_t new_circuit_capacity_checker();
void reset_circuit_capacity_checker(uint64_t id);
char* apply_tx(uint64_t id, char *tx_traces);
char* apply_block(uint64_t id, char *block_trace);
char* get_tx_num(uint64_t id);
50 changes: 50 additions & 0 deletions rollup/circuitcapacitychecker/libzkp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ pub mod checker {
pub error: Option<String>,
}

#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct TxNumResult {
pub tx_num: u64,
pub error: Option<String>,
}

static mut CHECKERS: OnceCell<HashMap<u64, CircuitCapacityChecker>> = OnceCell::new();

/// # Safety
Expand Down Expand Up @@ -176,6 +182,50 @@ pub mod checker {
}
}
}

/// # Safety
#[no_mangle]
pub unsafe extern "C" fn get_tx_num(id: u64) -> *const c_char {
let result = get_tx_num_inner(id);
let r = match result {
Ok(tx_num) => {
log::debug!("id: {id}, tx_num: {tx_num}");
TxNumResult {
tx_num,
error: None,
}
}
Err(e) => TxNumResult {
tx_num: 0,
error: Some(format!("{e:?}")),
},
};
serde_json::to_vec(&r).map_or(null(), vec_to_c_char)
}

unsafe fn get_tx_num_inner(id: u64) -> Result<u64, Error> {
log::debug!("ccc get_tx_num raw input, id: {id}");
panic::catch_unwind(|| {
Ok(CHECKERS
.get_mut()
.ok_or(anyhow!(
"fail to get circuit capacity checkers map in get_tx_num"
))?
.get_mut(&id)
.ok_or(anyhow!(
"fail to get circuit capacity checker (id: {id}) in get_tx_num"
))?
.get_tx_num() as u64)
})
.map_or_else(
|e| {
Err(anyhow!(
"circuit capacity checker (id: {id}) error in get_tx_num: {e:?}"
))
},
|result| result,
)
}
}

pub(crate) mod utils {
Expand Down
13 changes: 13 additions & 0 deletions rollup/circuitcapacitychecker/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ type CircuitCapacityChecker struct {
nextError *error
}

// NewCircuitCapacityChecker creates a new CircuitCapacityChecker
func NewCircuitCapacityChecker() *CircuitCapacityChecker {
return &CircuitCapacityChecker{ID: rand.Uint64()}
}

// Reset resets a ccc, but need to do nothing in mock_ccc.
func (ccc *CircuitCapacityChecker) Reset() {
}

// ApplyTransaction appends a tx's wrapped BlockTrace into the ccc, and return the accumulated RowConsumption.
// Will only return a dummy value in mock_ccc.
func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*types.RowConsumption, error) {
if ccc.nextError != nil {
ccc.countdown--
Expand All @@ -36,13 +40,22 @@ func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*
}}, nil
}

// ApplyBlock gets a block's RowConsumption.
// Will only return a dummy value in mock_ccc.
func (ccc *CircuitCapacityChecker) ApplyBlock(traces *types.BlockTrace) (*types.RowConsumption, error) {
return &types.RowConsumption{types.SubCircuitRowUsage{
Name: "mock",
RowNumber: 2,
}}, nil
}

// CheckTxNum compares whether the tx_count in ccc match the expected.
// Will alway return true in mock_ccc.
func (ccc *CircuitCapacityChecker) CheckTxNum(expected int) (bool, uint64, error) {
return true, uint64(expected), nil
}

// ScheduleError schedules an error for a tx (see `ApplyTransaction`), only used in tests.
func (ccc *CircuitCapacityChecker) ScheduleError(cnt int, err error) {
ccc.countdown = cnt
ccc.nextError = &err
Expand Down
5 changes: 5 additions & 0 deletions rollup/circuitcapacitychecker/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@ type WrappedRowUsage struct {
AccRowUsage *types.RowUsage `json:"acc_row_usage,omitempty"`
Error string `json:"error,omitempty"`
}

type WrappedTxNum struct {
TxNum uint64 `json:"tx_num"`
Error string `json:"error,omitempty"`
}

0 comments on commit 7d79f27

Please sign in to comment.