From 02c2ee155a65f6edcf0c322c309ae73aa7916e95 Mon Sep 17 00:00:00 2001 From: Eduard S Date: Mon, 28 Dec 2020 16:42:30 +0100 Subject: [PATCH] Fix item_id order in forged l1UserTxs Also, make sure that all SQL queries that return slices are sorted --- db/historydb/historydb.go | 29 ++++++---- db/historydb/historydb_test.go | 101 +++++++++++++++++++++++++++++++++ db/migrations/0001.sql | 9 +-- synchronizer/synchronizer.go | 7 ++- 4 files changed, 127 insertions(+), 19 deletions(-) diff --git a/db/historydb/historydb.go b/db/historydb/historydb.go index 799d04a..8b792f8 100644 --- a/db/historydb/historydb.go +++ b/db/historydb/historydb.go @@ -82,7 +82,7 @@ func (hdb *HistoryDB) GetAllBlocks() ([]common.Block, error) { var blocks []*common.Block err := meddler.QueryAll( hdb.db, &blocks, - "SELECT * FROM block;", + "SELECT * FROM block ORDER BY eth_block_num;", ) return db.SlicePtrsToSlice(blocks).([]common.Block), tracerr.Wrap(err) } @@ -92,7 +92,7 @@ func (hdb *HistoryDB) GetBlocks(from, to int64) ([]common.Block, error) { var blocks []*common.Block err := meddler.QueryAll( hdb.db, &blocks, - "SELECT * FROM block WHERE $1 <= eth_block_num AND eth_block_num < $2;", + "SELECT * FROM block WHERE $1 <= eth_block_num AND eth_block_num < $2 ORDER BY eth_block_num;", from, to, ) return db.SlicePtrsToSlice(blocks).([]common.Block), tracerr.Wrap(err) @@ -277,7 +277,8 @@ func (hdb *HistoryDB) GetAllBatches() ([]common.Batch, error) { hdb.db, &batches, `SELECT batch.batch_num, batch.eth_block_num, batch.forger_addr, batch.fees_collected, batch.fee_idxs_coordinator, batch.state_root, batch.num_accounts, batch.last_idx, batch.exit_root, - batch.forge_l1_txs_num, batch.slot_num, batch.total_fees_usd FROM batch;`, + batch.forge_l1_txs_num, batch.slot_num, batch.total_fees_usd FROM batch + ORDER BY item_id;`, ) return db.SlicePtrsToSlice(batches).([]common.Batch), tracerr.Wrap(err) } @@ -287,7 +288,7 @@ func (hdb *HistoryDB) GetBatches(from, to common.BatchNum) ([]common.Batch, erro var batches []*common.Batch err := meddler.QueryAll( hdb.db, &batches, - "SELECT * FROM batch WHERE $1 <= batch_num AND batch_num < $2;", + "SELECT * FROM batch WHERE $1 <= batch_num AND batch_num < $2 ORDER BY batch_num;", from, to, ) return db.SlicePtrsToSlice(batches).([]common.Batch), tracerr.Wrap(err) @@ -360,7 +361,8 @@ func (hdb *HistoryDB) GetAllBids() ([]common.Bid, error) { var bids []*common.Bid err := meddler.QueryAll( hdb.db, &bids, - `SELECT bid.slot_num, bid.bid_value, bid.eth_block_num, bid.bidder_addr FROM bid;`, + `SELECT bid.slot_num, bid.bid_value, bid.eth_block_num, bid.bidder_addr FROM bid + ORDER BY item_id;`, ) return db.SlicePtrsToSlice(bids).([]common.Bid), tracerr.Wrap(err) } @@ -805,6 +807,9 @@ func (hdb *HistoryDB) AddL1Txs(l1txs []common.L1Tx) error { // BatchNum should be null, and the value will be setted by a trigger when a batch forges the tx. // EffectiveAmount and EffectiveDepositAmount are seted with default values by the DB. func (hdb *HistoryDB) addL1Txs(d meddler.DB, l1txs []common.L1Tx) error { + if len(l1txs) == 0 { + return nil + } txs := []txWrite{} for i := 0; i < len(l1txs); i++ { af := new(big.Float).SetInt(l1txs[i].Amount) @@ -1063,7 +1068,7 @@ func (hdb *HistoryDB) GetAllExits() ([]common.ExitInfo, error) { hdb.db, &exits, `SELECT exit_tree.batch_num, exit_tree.account_idx, exit_tree.merkle_proof, exit_tree.balance, exit_tree.instant_withdrawn, exit_tree.delayed_withdraw_request, - exit_tree.delayed_withdrawn FROM exit_tree;`, + exit_tree.delayed_withdrawn FROM exit_tree ORDER BY item_id;`, ) return db.SlicePtrsToSlice(exits).([]common.ExitInfo), tracerr.Wrap(err) } @@ -1207,7 +1212,7 @@ func (hdb *HistoryDB) GetAllL1UserTxs() ([]common.L1Tx, error) { 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 - FROM tx WHERE is_l1 = TRUE AND user_origin = TRUE;`, + FROM tx WHERE is_l1 = TRUE AND user_origin = TRUE ORDER BY item_id;`, ) return db.SlicePtrsToSlice(txs).([]common.L1Tx), tracerr.Wrap(err) } @@ -1224,7 +1229,7 @@ func (hdb *HistoryDB) GetAllL1CoordinatorTxs() ([]common.L1Tx, error) { 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 - FROM tx WHERE is_l1 = TRUE AND user_origin = FALSE;`, + FROM tx WHERE is_l1 = TRUE AND user_origin = FALSE ORDER BY item_id;`, ) return db.SlicePtrsToSlice(txs).([]common.L1Tx), tracerr.Wrap(err) } @@ -1237,7 +1242,7 @@ func (hdb *HistoryDB) GetAllL2Txs() ([]common.L2Tx, error) { `SELECT tx.id, tx.batch_num, tx.position, tx.from_idx, tx.to_idx, tx.amount, tx.fee, tx.nonce, tx.type, tx.eth_block_num - FROM tx WHERE is_l1 = FALSE;`, + FROM tx WHERE is_l1 = FALSE ORDER BY item_id;`, ) return db.SlicePtrsToSlice(txs).([]common.L2Tx), tracerr.Wrap(err) } @@ -1325,7 +1330,7 @@ func (hdb *HistoryDB) GetAllBucketUpdates() ([]common.BucketUpdate, error) { var bucketUpdates []*common.BucketUpdate err := meddler.QueryAll( hdb.db, &bucketUpdates, - "SELECT * FROM bucket_update;", + "SELECT * FROM bucket_update ORDER BY item_id;", ) return db.SlicePtrsToSlice(bucketUpdates).([]common.BucketUpdate), tracerr.Wrap(err) } @@ -1350,7 +1355,7 @@ func (hdb *HistoryDB) GetAllTokenExchanges() ([]common.TokenExchange, error) { var tokenExchanges []*common.TokenExchange err := meddler.QueryAll( hdb.db, &tokenExchanges, - "SELECT * FROM token_exchange;", + "SELECT * FROM token_exchange ORDER BY item_id;", ) return db.SlicePtrsToSlice(tokenExchanges).([]common.TokenExchange), tracerr.Wrap(err) } @@ -1378,7 +1383,7 @@ func (hdb *HistoryDB) GetAllEscapeHatchWithdrawals() ([]common.WDelayerEscapeHat var escapeHatchWithdrawals []*common.WDelayerEscapeHatchWithdrawal err := meddler.QueryAll( hdb.db, &escapeHatchWithdrawals, - "SELECT * FROM escape_hatch_withdrawal;", + "SELECT * FROM escape_hatch_withdrawal ORDER BY item_id;", ) return db.SlicePtrsToSlice(escapeHatchWithdrawals).([]common.WDelayerEscapeHatchWithdrawal), tracerr.Wrap(err) diff --git a/db/historydb/historydb_test.go b/db/historydb/historydb_test.go index f5dbc4d..6faccdd 100644 --- a/db/historydb/historydb_test.go +++ b/db/historydb/historydb_test.go @@ -2,6 +2,7 @@ package historydb import ( "database/sql" + "fmt" "math" "math/big" "os" @@ -1115,6 +1116,106 @@ func TestGetFirstBatchBlockNumBySlot(t *testing.T) { assert.Equal(t, int64(10), bn2) } +func TestTxItemID(t *testing.T) { + test.WipeDB(historyDB.DB()) + testUsersLen := 10 + var set []til.Instruction + for user := 0; user < testUsersLen; user++ { + set = append(set, til.Instruction{ + Typ: common.TxTypeCreateAccountDeposit, + TokenID: common.TokenID(0), + DepositAmount: big.NewInt(1000000), + Amount: big.NewInt(0), + From: fmt.Sprintf("User%02d", user), + }) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + } + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + for user := 0; user < testUsersLen; user++ { + set = append(set, til.Instruction{ + Typ: common.TxTypeDeposit, + TokenID: common.TokenID(0), + DepositAmount: big.NewInt(100000), + Amount: big.NewInt(0), + From: fmt.Sprintf("User%02d", user), + }) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + } + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + for user := 0; user < testUsersLen; user++ { + set = append(set, til.Instruction{ + Typ: common.TxTypeDepositTransfer, + TokenID: common.TokenID(0), + DepositAmount: big.NewInt(10000 * int64(user+1)), + Amount: big.NewInt(1000 * int64(user+1)), + From: fmt.Sprintf("User%02d", user), + To: fmt.Sprintf("User%02d", (user+1)%testUsersLen), + }) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + } + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + for user := 0; user < testUsersLen; user++ { + set = append(set, til.Instruction{ + Typ: common.TxTypeForceTransfer, + TokenID: common.TokenID(0), + Amount: big.NewInt(100 * int64(user+1)), + DepositAmount: big.NewInt(0), + From: fmt.Sprintf("User%02d", user), + To: fmt.Sprintf("User%02d", (user+1)%testUsersLen), + }) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + } + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + for user := 0; user < testUsersLen; user++ { + set = append(set, til.Instruction{ + Typ: common.TxTypeForceExit, + TokenID: common.TokenID(0), + Amount: big.NewInt(10 * int64(user+1)), + DepositAmount: big.NewInt(0), + From: fmt.Sprintf("User%02d", user), + }) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + } + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBatchL1}) + set = append(set, til.Instruction{Typ: til.TypeNewBlock}) + var chainID uint16 = 0 + tc := til.NewContext(chainID, common.RollupConstMaxL1UserTx) + blocks, err := tc.GenerateBlocksFromInstructions(set) + assert.NoError(t, err) + + tilCfgExtra := til.ConfigExtra{ + CoordUser: "A", + } + err = tc.FillBlocksExtra(blocks, &tilCfgExtra) + require.NoError(t, err) + + // Add all blocks + for i := range blocks { + err = historyDB.AddBlockSCData(&blocks[i]) + require.NoError(t, err) + } + + txs, err := historyDB.GetAllL1UserTxs() + require.NoError(t, err) + position := 0 + for _, tx := range txs { + if tx.Position == 0 { + position = 0 + } + assert.Equal(t, position, tx.Position) + position++ + } +} + // setTestBlocks WARNING: this will delete the blocks and recreate them func setTestBlocks(from, to int64) []common.Block { test.WipeDB(historyDB.DB()) diff --git a/db/migrations/0001.sql b/db/migrations/0001.sql index 1794958..6fef4ff 100644 --- a/db/migrations/0001.sql +++ b/db/migrations/0001.sql @@ -515,13 +515,14 @@ $BODY$ BEGIN IF NEW.forge_l1_txs_num IS NOT NULL THEN UPDATE tx - SET item_id = nextval('tx_item_id'), batch_num = NEW.batch_num - WHERE id IN ( - SELECT id FROM tx + SET item_id = upd.item_id, batch_num = NEW.batch_num + FROM ( + SELECT id, nextval('tx_item_id') FROM tx WHERE user_origin AND NEW.forge_l1_txs_num = to_forge_l1_txs_num ORDER BY position FOR UPDATE - ); + ) as upd (id, item_id) + WHERE tx.id = upd.id; END IF; RETURN NEW; END; diff --git a/synchronizer/synchronizer.go b/synchronizer/synchronizer.go index 598dc93..9155020 100644 --- a/synchronizer/synchronizer.go +++ b/synchronizer/synchronizer.go @@ -318,9 +318,10 @@ func (s *Synchronizer) updateCurrentSlotIfSync(reset bool, firstBatchBlockNum *i } else if err == nil { slot.BidValue = bidCoord.BidValue slot.DefaultSlotBid = bidCoord.DefaultSlotSetBid[slot.SlotNum%6] - // Only if the highest bid value is higher than the - // default slot bid, the bidder is the winner of the - // slot. Otherwise the boot coordinator is the winner. + // Only if the highest bid value is greater/equal than + // the default slot bid, the bidder is the winner of + // the slot. Otherwise the boot coordinator is the + // winner. if slot.BidValue.Cmp(slot.DefaultSlotBid) >= 0 { slot.Bidder = bidCoord.Bidder slot.Forger = bidCoord.Forger