From ec194d5066bf518cc78aaee4fefb42cb08bd5bb8 Mon Sep 17 00:00:00 2001 From: Eduard S Date: Wed, 13 Jan 2021 14:51:59 +0100 Subject: [PATCH] Set l1tx.EffectiveFromIdx in TxProcessor and til --- api/api_test.go | 6 +++++- api/txshistory_test.go | 11 +++------- common/l1tx.go | 21 +++++++++--------- db/historydb/historydb.go | 36 ++++++++++++++++++------------- db/historydb/historydb_test.go | 5 +++-- node/node.go | 1 + synchronizer/synchronizer.go | 21 +++++++++++------- synchronizer/synchronizer_test.go | 16 ++++++++++++++ test/til/txs.go | 31 ++++++++++++++++++++------ txprocessor/txprocessor.go | 19 +++++++++++----- 10 files changed, 112 insertions(+), 55 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index 6171185..86cba92 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -268,6 +268,10 @@ func TestMain(m *testing.M) { if err != nil { panic(err) } + err = tcc.FillBlocksForgedL1UserTxs(blocksData) + if err != nil { + panic(err) + } AddAditionalInformation(blocksData) // Generate L2 Txs with til commonPoolTxs, err := tcc.GeneratePoolL2Txs(txsets.SetPoolL2MinimumFlow0) @@ -330,7 +334,6 @@ func TestMain(m *testing.M) { testTokens = append(testTokens, token) } // Set USD value for tokens in DB - commonL1Txs = append(commonL1Txs, block.Rollup.L1UserTxs...) for _, batch := range block.Rollup.Batches { commonL2Txs = append(commonL2Txs, batch.L2Txs...) for i := range batch.CreatedAccounts { @@ -339,6 +342,7 @@ func TestMain(m *testing.M) { } commonBatches = append(commonBatches, batch.Batch) commonExitTree = append(commonExitTree, batch.ExitTree...) + commonL1Txs = append(commonL1Txs, batch.L1UserTxs...) commonL1Txs = append(commonL1Txs, batch.L1CoordinatorTxs...) } } diff --git a/api/txshistory_test.go b/api/txshistory_test.go index 76195c4..4a35890 100644 --- a/api/txshistory_test.go +++ b/api/txshistory_test.go @@ -143,14 +143,9 @@ func genTestTxs( bn := common.BatchNum(*tx.L1Info.ToForgeL1TxsNum + 2) tx.BatchNum = &bn } - // TODO: User L1 txs that create txs will have fromAccountIndex equal to the idx of the - // created account. Once this is done this test will be broken and will need to be updated here. - // At the moment they are null - if l1.Type != common.TxTypeCreateAccountDeposit && - l1.Type != common.TxTypeCreateAccountDepositTransfer { - idxStr := idxToHez(l1.FromIdx, token.Symbol) - tx.FromIdx = &idxStr - } + // If FromIdx is not nil + idxStr := idxToHez(l1.EffectiveFromIdx, token.Symbol) + tx.FromIdx = &idxStr // If tx has a normal ToIdx (>255), set FromEthAddr and FromBJJ if l1.ToIdx >= common.UserThreshold { // find account diff --git a/common/l1tx.go b/common/l1tx.go index 9c86fb3..7025c15 100644 --- a/common/l1tx.go +++ b/common/l1tx.go @@ -28,16 +28,17 @@ type L1Tx struct { // where type: // - L1UserTx: 0 // - L1CoordinatorTx: 1 - TxID TxID `meddler:"id"` - ToForgeL1TxsNum *int64 `meddler:"to_forge_l1_txs_num"` // toForgeL1TxsNum in which the tx was forged / will be forged - Position int `meddler:"position"` - UserOrigin bool `meddler:"user_origin"` // true if the tx was originated by a user, false if it was aoriginated by a coordinator. Note that this differ from the spec for implementation simplification purpposes - FromIdx Idx `meddler:"from_idx,zeroisnull"` // FromIdx is used by L1Tx/Deposit to indicate the Idx receiver of the L1Tx.DepositAmount (deposit) - FromEthAddr ethCommon.Address `meddler:"from_eth_addr,zeroisnull"` - FromBJJ babyjub.PublicKeyComp `meddler:"from_bjj,zeroisnull"` - ToIdx Idx `meddler:"to_idx"` // ToIdx is ignored in L1Tx/Deposit, but used in the L1Tx/DepositAndTransfer - TokenID TokenID `meddler:"token_id"` - Amount *big.Int `meddler:"amount,bigint"` + TxID TxID `meddler:"id"` + ToForgeL1TxsNum *int64 `meddler:"to_forge_l1_txs_num"` // toForgeL1TxsNum in which the tx was forged / will be forged + Position int `meddler:"position"` + UserOrigin bool `meddler:"user_origin"` // true if the tx was originated by a user, false if it was aoriginated by a coordinator. Note that this differ from the spec for implementation simplification purpposes + FromIdx Idx `meddler:"from_idx,zeroisnull"` // FromIdx is used by L1Tx/Deposit to indicate the Idx receiver of the L1Tx.DepositAmount (deposit) + EffectiveFromIdx Idx `meddler:"effective_from_idx,zeroisnull"` + FromEthAddr ethCommon.Address `meddler:"from_eth_addr,zeroisnull"` + FromBJJ babyjub.PublicKeyComp `meddler:"from_bjj,zeroisnull"` + ToIdx Idx `meddler:"to_idx"` // ToIdx is ignored in L1Tx/Deposit, but used in the L1Tx/DepositAndTransfer + TokenID TokenID `meddler:"token_id"` + Amount *big.Int `meddler:"amount,bigint"` // EffectiveAmount only applies to L1UserTx. EffectiveAmount *big.Int `meddler:"effective_amount,bigintnull"` DepositAmount *big.Int `meddler:"deposit_amount,bigint"` diff --git a/db/historydb/historydb.go b/db/historydb/historydb.go index 05fb13f..c251cb5 100644 --- a/db/historydb/historydb.go +++ b/db/historydb/historydb.go @@ -852,9 +852,13 @@ func (hdb *HistoryDB) addL1Txs(d meddler.DB, l1txs []common.L1Tx) error { laf := new(big.Float).SetInt(l1txs[i].DepositAmount) depositAmountFloat, _ := laf.Float64() var effectiveFromIdx *common.Idx - if l1txs[i].Type != common.TxTypeCreateAccountDeposit && - l1txs[i].Type != common.TxTypeCreateAccountDepositTransfer { - effectiveFromIdx = &l1txs[i].FromIdx + if l1txs[i].UserOrigin { + if l1txs[i].Type != common.TxTypeCreateAccountDeposit && + l1txs[i].Type != common.TxTypeCreateAccountDepositTransfer { + effectiveFromIdx = &l1txs[i].FromIdx + } + } else { + effectiveFromIdx = &l1txs[i].EffectiveFromIdx } txs = append(txs, txWrite{ // Generic @@ -1242,7 +1246,7 @@ func (hdb *HistoryDB) GetAllL1UserTxs() ([]common.L1Tx, error) { err := meddler.QueryAll( hdb.db, &txs, // Note that '\x' gets parsed as a big.Int with value = 0 `SELECT tx.id, tx.to_forge_l1_txs_num, tx.position, tx.user_origin, - tx.from_idx, tx.from_eth_addr, tx.from_bjj, tx.to_idx, tx.token_id, + tx.from_idx, tx.effective_from_idx, tx.from_eth_addr, tx.from_bjj, tx.to_idx, tx.token_id, tx.amount, (CASE WHEN tx.batch_num IS NULL THEN NULL WHEN tx.amount_success THEN tx.amount ELSE '\x' END) AS effective_amount, tx.deposit_amount, (CASE WHEN tx.batch_num IS NULL THEN NULL WHEN tx.deposit_amount_success THEN tx.deposit_amount ELSE '\x' END) AS effective_deposit_amount, tx.eth_block_num, tx.type, tx.batch_num @@ -1259,7 +1263,7 @@ func (hdb *HistoryDB) GetAllL1CoordinatorTxs() ([]common.L1Tx, error) { err := meddler.QueryAll( hdb.db, &txs, `SELECT tx.id, tx.to_forge_l1_txs_num, tx.position, tx.user_origin, - tx.from_idx, tx.from_eth_addr, tx.from_bjj, tx.to_idx, tx.token_id, + tx.from_idx, tx.effective_from_idx, tx.from_eth_addr, tx.from_bjj, tx.to_idx, tx.token_id, tx.amount, tx.amount AS effective_amount, tx.deposit_amount, tx.deposit_amount AS effective_deposit_amount, tx.eth_block_num, tx.type, tx.batch_num @@ -1477,20 +1481,21 @@ func (hdb *HistoryDB) SetInitialSCVars(rollup *common.RollupVariables, return tracerr.Wrap(txn.Commit()) } -// setExtraInfoForgedL1UserTxs sets the EffectiveAmount and EffectiveDepositAmount -// of the given l1UserTxs (with an UPDATE) -// TODO: Set effective_from_idx for txs that create accounts +// setExtraInfoForgedL1UserTxs sets the EffectiveAmount, EffectiveDepositAmount +// and EffectiveFromIdx of the given l1UserTxs (with an UPDATE) func (hdb *HistoryDB) setExtraInfoForgedL1UserTxs(d sqlx.Ext, txs []common.L1Tx) error { if len(txs) == 0 { return nil } // Effective amounts are stored as success flags in the DB, with true value by default // to reduce the amount of updates. Therefore, only amounts that became uneffective should be - // updated to become false + // updated to become false. At the same time, all the txs that contain + // accounts (FromIdx == 0) are updated to set the EffectiveFromIdx. type txUpdate struct { ID common.TxID `db:"id"` AmountSuccess bool `db:"amount_success"` DepositAmountSuccess bool `db:"deposit_amount_success"` + EffectiveFromIdx common.Idx `db:"effective_from_idx"` } txUpdates := []txUpdate{} equal := func(a *big.Int, b *big.Int) bool { @@ -1499,22 +1504,24 @@ func (hdb *HistoryDB) setExtraInfoForgedL1UserTxs(d sqlx.Ext, txs []common.L1Tx) for i := range txs { amountSuccess := equal(txs[i].Amount, txs[i].EffectiveAmount) depositAmountSuccess := equal(txs[i].DepositAmount, txs[i].EffectiveDepositAmount) - if !amountSuccess || !depositAmountSuccess { + if !amountSuccess || !depositAmountSuccess || txs[i].FromIdx == 0 { txUpdates = append(txUpdates, txUpdate{ ID: txs[i].TxID, AmountSuccess: amountSuccess, DepositAmountSuccess: depositAmountSuccess, + EffectiveFromIdx: txs[i].EffectiveFromIdx, }) } } const query string = ` UPDATE tx SET amount_success = tx_update.amount_success, - deposit_amount_success = tx_update.deposit_amount_success + deposit_amount_success = tx_update.deposit_amount_success, + effective_from_idx = tx_update.effective_from_idx FROM (VALUES - (NULL::::BYTEA, NULL::::BOOL, NULL::::BOOL), - (:id, :amount_success, :deposit_amount_success) - ) as tx_update (id, amount_success, deposit_amount_success) + (NULL::::BYTEA, NULL::::BOOL, NULL::::BOOL, NULL::::BIGINT), + (:id, :amount_success, :deposit_amount_success, :effective_from_idx) + ) as tx_update (id, amount_success, deposit_amount_success, effective_from_idx) WHERE tx.id = tx_update.id; ` if len(txUpdates) > 0 { @@ -1604,7 +1611,6 @@ func (hdb *HistoryDB) AddBlockSCData(blockData *common.BlockData) (err error) { // Set the EffectiveAmount and EffectiveDepositAmount of all the // L1UserTxs that have been forged in this batch - // TODO: Set also effective_from_idx for txs that create accounts if err = hdb.setExtraInfoForgedL1UserTxs(txn, batch.L1UserTxs); err != nil { return tracerr.Wrap(err) } diff --git a/db/historydb/historydb_test.go b/db/historydb/historydb_test.go index 43034ad..4bd2549 100644 --- a/db/historydb/historydb_test.go +++ b/db/historydb/historydb_test.go @@ -791,9 +791,10 @@ func TestSetExtraInfoForgedL1UserTxs(t *testing.T) { // Add second batch to trigger the update of the batch_num, // while avoiding the implicit call of setExtraInfoForgedL1UserTxs err = historyDB.addBlock(historyDB.db, &blocks[1].Block) - assert.NoError(t, err) + require.NoError(t, err) err = historyDB.addBatch(historyDB.db, &blocks[1].Rollup.Batches[0].Batch) - assert.NoError(t, err) + require.NoError(t, err) + err = historyDB.addAccounts(historyDB.db, blocks[1].Rollup.Batches[0].CreatedAccounts) require.NoError(t, err) // Set the Effective{Amount,DepositAmount} of the L1UserTxs that are forged in the second block diff --git a/node/node.go b/node/node.go index 2ee8789..7bc8db7 100644 --- a/node/node.go +++ b/node/node.go @@ -172,6 +172,7 @@ func NewNode(mode Mode, cfg *config.Node) (*Node, error) { sync, err := synchronizer.NewSynchronizer(client, historyDB, stateDB, synchronizer.Config{ StatsRefreshPeriod: cfg.Synchronizer.StatsRefreshPeriod.Duration, + ChainID: chainIDU16, }) if err != nil { return nil, tracerr.Wrap(err) diff --git a/synchronizer/synchronizer.go b/synchronizer/synchronizer.go index 88d94a7..3f7b2bb 100644 --- a/synchronizer/synchronizer.go +++ b/synchronizer/synchronizer.go @@ -180,6 +180,7 @@ type SCConsts struct { // Config is the Synchronizer configuration type Config struct { StatsRefreshPeriod time.Duration + ChainID uint16 } // Synchronizer implements the Synchronizer type @@ -797,17 +798,21 @@ func (s *Synchronizer) rollupSync(ethBlock *common.Block) (*common.RollupData, e // Transform L2 txs to PoolL2Txs poolL2Txs := common.L2TxsToPoolL2Txs(forgeBatchArgs.L2TxsData) // NOTE: This is a big ugly, find a better way - // ProcessTxs updates poolL2Txs adding: Nonce (and also TokenID, but we don't use it). - //nolint:gomnd - tpc := txprocessor.Config{ // TODO TMP - NLevels: 32, - MaxFeeTx: 64, - MaxTx: 512, - MaxL1Tx: 64, - ChainID: uint16(0), + if int(forgeBatchArgs.VerifierIdx) >= len(s.consts.Rollup.Verifiers) { + return nil, tracerr.Wrap(fmt.Errorf("forgeBatchArgs.VerifierIdx (%v) >= "+ + " len(s.consts.Rollup.Verifiers) (%v)", + forgeBatchArgs.VerifierIdx, len(s.consts.Rollup.Verifiers))) + } + tpc := txprocessor.Config{ + NLevels: uint32(s.consts.Rollup.Verifiers[forgeBatchArgs.VerifierIdx].NLevels), + MaxTx: uint32(s.consts.Rollup.Verifiers[forgeBatchArgs.VerifierIdx].MaxTx), + ChainID: s.cfg.ChainID, + MaxFeeTx: common.RollupConstMaxFeeIdxCoordinator, + MaxL1Tx: common.RollupConstMaxL1Tx, } tp := txprocessor.NewTxProcessor(s.stateDB, tpc) + // ProcessTxs updates poolL2Txs adding: Nonce (and also TokenID, but we don't use it). processTxsOut, err := tp.ProcessTxs(forgeBatchArgs.FeeIdxCoordinator, l1UserTxs, batchData.L1CoordinatorTxs, poolL2Txs) if err != nil { diff --git a/synchronizer/synchronizer_test.go b/synchronizer/synchronizer_test.go index e64b53b..80492dd 100644 --- a/synchronizer/synchronizer_test.go +++ b/synchronizer/synchronizer_test.go @@ -98,6 +98,11 @@ func checkSyncBlock(t *testing.T, s *Synchronizer, blockNum int, block, syncBloc tx.Position == _dbTx.Position { dbTx = new(common.L1Tx) *dbTx = _dbTx + // NOTE: Overwrite EffectiveFromIdx in L1UserTx + // from db because we don't expect + // EffectiveFromIdx to be set yet, as this tx + // is not in yet forged + dbTx.EffectiveFromIdx = 0 break } } @@ -142,7 +147,18 @@ func checkSyncBlock(t *testing.T, s *Synchronizer, blockNum int, block, syncBloc batch.Batch.NumAccounts = len(batch.CreatedAccounts) // Test field by field to facilitate debugging of errors + assert.Equal(t, len(batch.L1UserTxs), len(syncBatch.L1UserTxs)) + // NOTE: EffectiveFromIdx is set to til L1UserTxs in + // `FillBlocksForgedL1UserTxs` function + for j := range syncBatch.L1UserTxs { + assert.NotEqual(t, 0, syncBatch.L1UserTxs[j].EffectiveFromIdx) + } assert.Equal(t, batch.L1UserTxs, syncBatch.L1UserTxs) + // NOTE: EffectiveFromIdx is set to til L1CoordinatorTxs in + // `FillBlocksExtra` function + for j := range syncBatch.L1CoordinatorTxs { + assert.NotEqual(t, 0, syncBatch.L1CoordinatorTxs[j].EffectiveFromIdx) + } assert.Equal(t, batch.L1CoordinatorTxs, syncBatch.L1CoordinatorTxs) assert.Equal(t, batch.L2Txs, syncBatch.L2Txs) // In exit tree, we only check AccountIdx and Balance, because diff --git a/test/til/txs.go b/test/til/txs.go index ced6681..541f58a 100644 --- a/test/til/txs.go +++ b/test/til/txs.go @@ -45,6 +45,7 @@ type contextExtra struct { toForgeL1TxsNum int64 nonces map[common.Idx]common.Nonce idx int + idxByTxID map[common.TxID]common.Idx } // Context contains the data of the test @@ -107,6 +108,7 @@ func NewContext(chainID uint16, rollupConstMaxL1UserTx int) *Context { toForgeL1TxsNum: 0, nonces: make(map[common.Idx]common.Nonce), idx: common.UserThreshold, + idxByTxID: make(map[common.TxID]common.Idx), }, } } @@ -762,7 +764,12 @@ func (tc *Context) FillBlocksL1UserTxsBatchNum(blocks []common.BlockData) { // FillBlocksForgedL1UserTxs fills the L1UserTxs of a batch with the L1UserTxs // that are forged in that batch. It always sets `EffectiveAmount` = `Amount` -// and `EffectiveDepositAmount` = `DepositAmount`. +// and `EffectiveDepositAmount` = `DepositAmount`. This function requires a +// previous call to `FillBlocksExtra`. +// - blocks[].Rollup.L1UserTxs[].BatchNum +// - blocks[].Rollup.L1UserTxs[].EffectiveAmount +// - blocks[].Rollup.L1UserTxs[].EffectiveDepositAmount +// - blocks[].Rollup.L1UserTxs[].EffectiveFromIdx func (tc *Context) FillBlocksForgedL1UserTxs(blocks []common.BlockData) error { for i := range blocks { block := &blocks[i] @@ -783,6 +790,11 @@ func (tc *Context) FillBlocksForgedL1UserTxs(blocks []common.BlockData) error { return tracerr.Wrap(err) } *tx = *_tx + if tx.FromIdx == 0 { + tx.EffectiveFromIdx = tc.extra.idxByTxID[tx.TxID] + } else { + tx.EffectiveFromIdx = tx.FromIdx + } } } } @@ -802,6 +814,7 @@ func (tc *Context) FillBlocksForgedL1UserTxs(blocks []common.BlockData) error { // - blocks[].Rollup.Batch.L1CoordinatorTxs[].Position // - blocks[].Rollup.Batch.L1CoordinatorTxs[].EffectiveAmount // - blocks[].Rollup.Batch.L1CoordinatorTxs[].EffectiveDepositAmount +// - blocks[].Rollup.Batch.L1CoordinatorTxs[].EffectiveFromIdx // - blocks[].Rollup.Batch.L2Txs[].TxID // - blocks[].Rollup.Batch.L2Txs[].Position // - blocks[].Rollup.Batch.L2Txs[].Nonce @@ -839,15 +852,17 @@ func (tc *Context) FillBlocksExtra(blocks []common.BlockData, cfg *ConfigExtra) block := &blocks[i] for j := range block.Rollup.Batches { batch := &block.Rollup.Batches[j] - l1Txs := []common.L1Tx{} + l1Txs := []*common.L1Tx{} if batch.L1Batch { - for _, tx := range tc.Queues[*batch.Batch.ForgeL1TxsNum] { - l1Txs = append(l1Txs, tx.L1Tx) + for k := range tc.Queues[*batch.Batch.ForgeL1TxsNum] { + l1Txs = append(l1Txs, &tc.Queues[*batch.Batch.ForgeL1TxsNum][k].L1Tx) } } - l1Txs = append(l1Txs, batch.L1CoordinatorTxs...) + for k := range batch.L1CoordinatorTxs { + l1Txs = append(l1Txs, &batch.L1CoordinatorTxs[k]) + } for k := range l1Txs { - tx := &l1Txs[k] + tx := l1Txs[k] if tx.Type == common.TxTypeCreateAccountDeposit || tx.Type == common.TxTypeCreateAccountDepositTransfer { user, ok := tc.UsersByIdx[tc.extra.idx] @@ -864,6 +879,10 @@ func (tc *Context) FillBlocksExtra(blocks []common.BlockData, cfg *ConfigExtra) Nonce: 0, Balance: big.NewInt(0), }) + if !tx.UserOrigin { + tx.EffectiveFromIdx = common.Idx(tc.extra.idx) + } + tc.extra.idxByTxID[tx.TxID] = common.Idx(tc.extra.idx) tc.extra.idx++ } } diff --git a/txprocessor/txprocessor.go b/txprocessor/txprocessor.go index 56abdf6..fcff480 100644 --- a/txprocessor/txprocessor.go +++ b/txprocessor/txprocessor.go @@ -154,10 +154,14 @@ func (tp *TxProcessor) ProcessTxs(coordIdxs []common.Idx, l1usertxs, l1coordinat if err != nil { return nil, tracerr.Wrap(err) } - if tp.s.Typ == statedb.TypeSynchronizer && createdAccount != nil { - createdAccounts = append(createdAccounts, *createdAccount) + if tp.s.Typ == statedb.TypeSynchronizer { + if createdAccount != nil { + createdAccounts = append(createdAccounts, *createdAccount) + l1usertxs[i].EffectiveFromIdx = createdAccount.Idx + } else { + l1usertxs[i].EffectiveFromIdx = l1usertxs[i].FromIdx + } } - if tp.zki != nil { l1TxData, err := l1usertxs[i].BytesGeneric() if err != nil { @@ -201,8 +205,13 @@ func (tp *TxProcessor) ProcessTxs(coordIdxs []common.Idx, l1usertxs, l1coordinat if exitIdx != nil { log.Error("Unexpected Exit in L1CoordinatorTx") } - if tp.s.Typ == statedb.TypeSynchronizer && createdAccount != nil { - createdAccounts = append(createdAccounts, *createdAccount) + if tp.s.Typ == statedb.TypeSynchronizer { + if createdAccount != nil { + createdAccounts = append(createdAccounts, *createdAccount) + l1coordinatortxs[i].EffectiveFromIdx = createdAccount.Idx + } else { + l1coordinatortxs[i].EffectiveFromIdx = l1coordinatortxs[i].FromIdx + } } if tp.zki != nil { l1TxData, err := l1coordinatortxs[i].BytesGeneric()