From 5fd258ee07c6524f5d8b9805caccdba4fd1a0cbd Mon Sep 17 00:00:00 2001 From: Arnau B Date: Thu, 3 Dec 2020 17:32:45 +0100 Subject: [PATCH] Use flags for effective amounts in SQL schema --- api/swagger.yml | 2 +- coordinator/coordinator.go | 2 +- db/historydb/historydb.go | 102 ++++++++++++++++-------------- db/historydb/historydb_test.go | 16 +++-- db/historydb/views.go | 37 ++++++----- db/migrations/0001.sql | 13 +++- synchronizer/synchronizer.go | 2 +- synchronizer/synchronizer_test.go | 3 +- 8 files changed, 100 insertions(+), 77 deletions(-) diff --git a/api/swagger.yml b/api/swagger.yml index 2d80590..59845c5 100644 --- a/api/swagger.yml +++ b/api/swagger.yml @@ -795,7 +795,7 @@ paths: description: Only include slots with `slotNum >= minSlotNum`. By default, `minSlotNum = 0`. schema: $ref: '#/components/schemas/SlotNum' - - name: maxSlothNum + - name: maxSlotNum in: query required: false description: Only include slots with `slotNum <= maxSlotNum`. diff --git a/coordinator/coordinator.go b/coordinator/coordinator.go index d1260bf..8b1ef76 100644 --- a/coordinator/coordinator.go +++ b/coordinator/coordinator.go @@ -694,7 +694,7 @@ func (p *Pipeline) forgeSendServerProof(ctx context.Context, batchNum common.Bat p.lastScheduledL1BatchBlockNum = p.stats.Eth.LastBlock.Num // 2a: L1+L2 txs p.lastForgeL1TxsNum++ - l1UserTxs, err := p.historyDB.GetL1UserTxs(p.lastForgeL1TxsNum) + l1UserTxs, err := p.historyDB.GetUnforgedL1UserTxs(p.lastForgeL1TxsNum) if err != nil { return nil, tracerr.Wrap(err) } diff --git a/db/historydb/historydb.go b/db/historydb/historydb.go index 5d937b1..8d072cf 100644 --- a/db/historydb/historydb.go +++ b/db/historydb/historydb.go @@ -770,11 +770,13 @@ func (hdb *HistoryDB) GetAllAccounts() ([]common.Account, error) { // AddL1Txs inserts L1 txs to the DB. USD and LoadAmountUSD will be set automatically before storing the tx. // If the tx is originated by a coordinator, BatchNum must be provided. If it's originated by a user, // BatchNum should be null, and the value will be setted by a trigger when a batch forges the tx. +// EffectiveAmount and EffectiveLoadAmount are seted with default values by the DB. func (hdb *HistoryDB) AddL1Txs(l1txs []common.L1Tx) error { return hdb.addL1Txs(hdb.db, l1txs) } // addL1Txs inserts L1 txs to the DB. USD and LoadAmountUSD will be set automatically before storing the tx. // If the tx is originated by a coordinator, BatchNum must be provided. If it's originated by a user, // BatchNum should be null, and the value will be setted by a trigger when a batch forges the tx. +// EffectiveAmount and EffectiveLoadAmount are seted with default values by the DB. func (hdb *HistoryDB) addL1Txs(d meddler.DB, l1txs []common.L1Tx) error { txs := []txWrite{} for i := 0; i < len(l1txs); i++ { @@ -784,26 +786,24 @@ func (hdb *HistoryDB) addL1Txs(d meddler.DB, l1txs []common.L1Tx) error { loadAmountFloat, _ := laf.Float64() txs = append(txs, txWrite{ // Generic - IsL1: true, - TxID: l1txs[i].TxID, - Type: l1txs[i].Type, - Position: l1txs[i].Position, - FromIdx: &l1txs[i].FromIdx, - ToIdx: l1txs[i].ToIdx, - Amount: l1txs[i].Amount, - EffectiveAmount: l1txs[i].EffectiveAmount, - AmountFloat: amountFloat, - TokenID: l1txs[i].TokenID, - BatchNum: l1txs[i].BatchNum, - EthBlockNum: l1txs[i].EthBlockNum, + IsL1: true, + TxID: l1txs[i].TxID, + Type: l1txs[i].Type, + Position: l1txs[i].Position, + FromIdx: &l1txs[i].FromIdx, + ToIdx: l1txs[i].ToIdx, + Amount: l1txs[i].Amount, + AmountFloat: amountFloat, + TokenID: l1txs[i].TokenID, + BatchNum: l1txs[i].BatchNum, + EthBlockNum: l1txs[i].EthBlockNum, // L1 - ToForgeL1TxsNum: l1txs[i].ToForgeL1TxsNum, - UserOrigin: &l1txs[i].UserOrigin, - FromEthAddr: &l1txs[i].FromEthAddr, - FromBJJ: l1txs[i].FromBJJ, - LoadAmount: l1txs[i].LoadAmount, - EffectiveLoadAmount: l1txs[i].EffectiveLoadAmount, - LoadAmountFloat: &loadAmountFloat, + ToForgeL1TxsNum: l1txs[i].ToForgeL1TxsNum, + UserOrigin: &l1txs[i].UserOrigin, + FromEthAddr: &l1txs[i].FromEthAddr, + FromBJJ: l1txs[i].FromBJJ, + LoadAmount: l1txs[i].LoadAmount, + LoadAmountFloat: &loadAmountFloat, }) } return hdb.addTxs(d, txs) @@ -849,7 +849,6 @@ func (hdb *HistoryDB) addTxs(d meddler.DB, txs []txWrite) error { from_idx, to_idx, amount, - effective_amount, amount_f, token_id, batch_num, @@ -859,7 +858,6 @@ func (hdb *HistoryDB) addTxs(d meddler.DB, txs []txWrite) error { from_eth_addr, from_bjj, load_amount, - effective_load_amount, load_amount_f, fee, nonce @@ -881,6 +879,7 @@ func (hdb *HistoryDB) addTxs(d meddler.DB, txs []txWrite) error { // GetHistoryTx returns a tx from the DB given a TxID func (hdb *HistoryDB) GetHistoryTx(txID common.TxID) (*TxAPI, error) { + // TODO: add success flags for L1s tx := &TxAPI{} err := meddler.QueryRow( hdb.db, tx, `SELECT tx.item_id, tx.is_l1, tx.id, tx.type, tx.position, @@ -906,6 +905,7 @@ func (hdb *HistoryDB) GetHistoryTxs( tokenID *common.TokenID, idx *common.Idx, batchNum *uint, txType *common.TxType, fromItem, limit *uint, order string, ) ([]TxAPI, uint64, error) { + // TODO: add success flags for L1s if ethAddr != nil && bjj != nil { return nil, 0, tracerr.Wrap(errors.New("ethAddr and bjj are incompatible")) } @@ -1165,10 +1165,11 @@ func (hdb *HistoryDB) GetExitsAPI( func (hdb *HistoryDB) GetAllL1UserTxs() ([]common.L1Tx, error) { var txs []*common.L1Tx err := meddler.QueryAll( - hdb.db, &txs, + 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.amount, tx.effective_amount, tx.load_amount, tx.effective_load_amount, + 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.load_amount, (CASE WHEN tx.batch_num IS NULL THEN NULL WHEN tx.load_amount_success THEN tx.load_amount ELSE '\x' END) AS effective_load_amount, tx.eth_block_num, tx.type, tx.batch_num FROM tx WHERE is_l1 = TRUE AND user_origin = TRUE;`, ) @@ -1178,11 +1179,14 @@ func (hdb *HistoryDB) GetAllL1UserTxs() ([]common.L1Tx, error) { // GetAllL1CoordinatorTxs returns all L1CoordinatorTxs from the DB func (hdb *HistoryDB) GetAllL1CoordinatorTxs() ([]common.L1Tx, error) { var txs []*common.L1Tx + // Since the query specifies that only coordinator txs are returned, it's safe to assume + // that returned txs will always have effective amounts 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.amount, tx.effective_amount, tx.load_amount, tx.effective_load_amount, + tx.amount, tx.amount AS effective_amount, + tx.load_amount, tx.load_amount AS effective_load_amount, tx.eth_block_num, tx.type, tx.batch_num FROM tx WHERE is_l1 = TRUE AND user_origin = FALSE;`, ) @@ -1202,16 +1206,17 @@ func (hdb *HistoryDB) GetAllL2Txs() ([]common.L2Tx, error) { return db.SlicePtrsToSlice(txs).([]common.L2Tx), tracerr.Wrap(err) } -// GetL1UserTxs gets L1 User Txs to be forged in the L1Batch with toForgeL1TxsNum. -func (hdb *HistoryDB) GetL1UserTxs(toForgeL1TxsNum int64) ([]common.L1Tx, error) { +// GetUnforgedL1UserTxs gets L1 User Txs to be forged in the L1Batch with toForgeL1TxsNum. +func (hdb *HistoryDB) GetUnforgedL1UserTxs(toForgeL1TxsNum int64) ([]common.L1Tx, error) { var txs []*common.L1Tx err := meddler.QueryAll( - hdb.db, &txs, + hdb.db, &txs, // only L1 user txs can have batch_num set to null `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.amount, tx.effective_amount, tx.load_amount, tx.effective_load_amount, + tx.amount, NULL AS effective_amount, + tx.load_amount, NULL AS effective_load_amount, tx.eth_block_num, tx.type, tx.batch_num - FROM tx WHERE to_forge_l1_txs_num = $1 AND is_l1 = TRUE AND user_origin = TRUE;`, + FROM tx WHERE batch_num IS NULL AND to_forge_l1_txs_num = $1;`, toForgeL1TxsNum, ) return db.SlicePtrsToSlice(txs).([]common.L1Tx), tracerr.Wrap(err) @@ -1296,39 +1301,40 @@ func (hdb *HistoryDB) SetInitialSCVars(rollup *common.RollupVariables, // setL1UserTxEffectiveAmounts sets the EffectiveAmount and EffectiveLoadAmount // of the given l1UserTxs (with an UPDATE) func (hdb *HistoryDB) setL1UserTxEffectiveAmounts(d sqlx.Ext, txs []common.L1Tx) error { + // 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 type txUpdate struct { - ID common.TxID `db:"id"` - NullifiedAmount bool `db:"nullified_amount"` - NullifiedLoadAmount bool `db:"nullified_load_amount"` + ID common.TxID `db:"id"` + AmountSuccess bool `db:"amount_success"` + LoadAmountSuccess bool `db:"load_amount_success"` } - txUpdates := make([]txUpdate, len(txs)) + txUpdates := []txUpdate{} equal := func(a *big.Int, b *big.Int) bool { return a.Cmp(b) == 0 } for i := range txs { - txUpdates[i] = txUpdate{ - ID: txs[i].TxID, - NullifiedAmount: !equal(txs[i].Amount, txs[i].EffectiveAmount), - NullifiedLoadAmount: !equal(txs[i].LoadAmount, txs[i].EffectiveLoadAmount), + amountSuccess := equal(txs[i].Amount, txs[i].EffectiveAmount) + loadAmountSuccess := equal(txs[i].LoadAmount, txs[i].EffectiveLoadAmount) + if !amountSuccess || !loadAmountSuccess { + txUpdates = append(txUpdates, txUpdate{ + ID: txs[i].TxID, + AmountSuccess: amountSuccess, + LoadAmountSuccess: loadAmountSuccess, + }) } } const query string = ` UPDATE tx SET - effective_amount = CASE - WHEN tx_update.nullified_amount THEN '\x' - ELSE tx.amount - END, - effective_load_amount = CASE - WHEN tx_update.nullified_load_amount THEN '\x' - ELSE tx.load_amount - END + amount_success = tx_update.amount_success, + load_amount_success = tx_update.load_amount_success FROM (VALUES (NULL::::BYTEA, NULL::::BOOL, NULL::::BOOL), - (:id, :nullified_amount, :nullified_load_amount) - ) as tx_update (id, nullified_amount, nullified_load_amount) + (:id, :amount_success, :load_amount_success) + ) as tx_update (id, amount_success, load_amount_success) WHERE tx.id = tx_update.id ` - if len(txs) > 0 { + if len(txUpdates) > 0 { if _, err := sqlx.NamedQuery(d, query, txUpdates); err != nil { return tracerr.Wrap(err) } diff --git a/db/historydb/historydb_test.go b/db/historydb/historydb_test.go index eb5495e..15ff2ba 100644 --- a/db/historydb/historydb_test.go +++ b/db/historydb/historydb_test.go @@ -598,7 +598,7 @@ func TestExitTree(t *testing.T) { assert.NoError(t, err) } -func TestGetL1UserTxs(t *testing.T) { +func TestGetUnforgedL1UserTxs(t *testing.T) { test.WipeDB(historyDB.DB()) set := ` @@ -629,13 +629,13 @@ func TestGetL1UserTxs(t *testing.T) { require.Nil(t, err) } - l1UserTxs, err := historyDB.GetL1UserTxs(toForgeL1TxsNum) + l1UserTxs, err := historyDB.GetUnforgedL1UserTxs(toForgeL1TxsNum) require.Nil(t, err) assert.Equal(t, 5, len(l1UserTxs)) assert.Equal(t, blocks[0].Rollup.L1UserTxs, l1UserTxs) // No l1UserTxs for this toForgeL1TxsNum - l1UserTxs, err = historyDB.GetL1UserTxs(2) + l1UserTxs, err = historyDB.GetUnforgedL1UserTxs(2) require.Nil(t, err) assert.Equal(t, 0, len(l1UserTxs)) } @@ -728,6 +728,13 @@ func TestSetL1UserTxEffectiveAmounts(t *testing.T) { err = historyDB.AddBlockSCData(&blocks[i]) require.Nil(t, err) } + // Add second batch to trigger the update of the batch_num, + // while avoiding the implicit call of setL1UserTxEffectiveAmounts + err = historyDB.addBlock(historyDB.db, &blocks[1].Block) + assert.NoError(t, err) + err = historyDB.addBatch(historyDB.db, &blocks[1].Rollup.Batches[0].Batch) + assert.NoError(t, err) + require.Nil(t, err) // Set the Effective{Amount,LoadAmount} of the L1UserTxs that are forged in the second block l1Txs := blocks[1].Rollup.Batches[0].L1UserTxs @@ -741,7 +748,8 @@ func TestSetL1UserTxEffectiveAmounts(t *testing.T) { dbL1Txs, err := historyDB.GetAllL1UserTxs() require.NoError(t, err) - for _, tx := range dbL1Txs { + for i, tx := range dbL1Txs { + log.Infof("%d %v %v", i, tx.EffectiveAmount, tx.EffectiveLoadAmount) assert.NotNil(t, tx.EffectiveAmount) assert.NotNil(t, tx.EffectiveLoadAmount) switch tx.TxID { diff --git a/db/historydb/views.go b/db/historydb/views.go index 2e18ad1..5a3887c 100644 --- a/db/historydb/views.go +++ b/db/historydb/views.go @@ -110,28 +110,27 @@ func (tx TxAPI) MarshalJSON() ([]byte, error) { // txWrite is an representatiion that merges common.L1Tx and common.L2Tx // in order to perform inserts into tx table +// EffectiveAmount and LoadEffectiveAmount are not set since they have default values in the DB type txWrite struct { // Generic - IsL1 bool `meddler:"is_l1"` - TxID common.TxID `meddler:"id"` - Type common.TxType `meddler:"type"` - Position int `meddler:"position"` - FromIdx *common.Idx `meddler:"from_idx"` - ToIdx common.Idx `meddler:"to_idx"` - Amount *big.Int `meddler:"amount,bigint"` - EffectiveAmount *big.Int `meddler:"effective_amount,bigintnull"` - AmountFloat float64 `meddler:"amount_f"` - TokenID common.TokenID `meddler:"token_id"` - BatchNum *common.BatchNum `meddler:"batch_num"` // batchNum in which this tx was forged. If the tx is L2, this must be != 0 - EthBlockNum int64 `meddler:"eth_block_num"` // Ethereum Block Number in which this L1Tx was added to the queue + IsL1 bool `meddler:"is_l1"` + TxID common.TxID `meddler:"id"` + Type common.TxType `meddler:"type"` + Position int `meddler:"position"` + FromIdx *common.Idx `meddler:"from_idx"` + ToIdx common.Idx `meddler:"to_idx"` + Amount *big.Int `meddler:"amount,bigint"` + AmountFloat float64 `meddler:"amount_f"` + TokenID common.TokenID `meddler:"token_id"` + BatchNum *common.BatchNum `meddler:"batch_num"` // batchNum in which this tx was forged. If the tx is L2, this must be != 0 + EthBlockNum int64 `meddler:"eth_block_num"` // Ethereum Block Number in which this L1Tx was added to the queue // L1 - ToForgeL1TxsNum *int64 `meddler:"to_forge_l1_txs_num"` // toForgeL1TxsNum in which the tx was forged / will be forged - 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 - FromEthAddr *ethCommon.Address `meddler:"from_eth_addr"` - FromBJJ *babyjub.PublicKey `meddler:"from_bjj"` - LoadAmount *big.Int `meddler:"load_amount,bigintnull"` - EffectiveLoadAmount *big.Int `meddler:"effective_load_amount,bigintnull"` - LoadAmountFloat *float64 `meddler:"load_amount_f"` + ToForgeL1TxsNum *int64 `meddler:"to_forge_l1_txs_num"` // toForgeL1TxsNum in which the tx was forged / will be forged + 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 + FromEthAddr *ethCommon.Address `meddler:"from_eth_addr"` + FromBJJ *babyjub.PublicKey `meddler:"from_bjj"` + LoadAmount *big.Int `meddler:"load_amount,bigintnull"` + LoadAmountFloat *float64 `meddler:"load_amount_f"` // L2 Fee *common.FeeSelector `meddler:"fee"` Nonce *common.Nonce `meddler:"nonce"` diff --git a/db/migrations/0001.sql b/db/migrations/0001.sql index 0ad25b6..604325b 100644 --- a/db/migrations/0001.sql +++ b/db/migrations/0001.sql @@ -135,6 +135,15 @@ FOR EACH ROW EXECUTE PROCEDURE set_token_usd_update(); CREATE SEQUENCE tx_item_id; +-- important note about "amount_success" and "load_amount_success" (only relevant to L1 user txs): +-- The behaviour should be: +-- When tx is not forged: amount_success = false, load_amount_success = false +-- When tx is forged: +-- amount_success = false if the "effective amount" is 0, else true +-- load_amount_success = false if the "effective load amount" is 0, else true +-- +-- However, in order to reduce the amount of updates, by default amount_success and load_amount_success will be set to true (when tx is unforged) +-- whne they should be false. This can be worked around at a query level by checking if "batch_num IS NULL" (which indicates that the tx is unforged). CREATE TABLE tx ( -- Generic TX item_id INTEGER PRIMARY KEY DEFAULT nextval('tx_item_id'), @@ -149,7 +158,7 @@ CREATE TABLE tx ( to_eth_addr BYTEA, to_bjj BYTEA, amount BYTEA NOT NULL, - effective_amount BYTEA, + amount_success BOOLEAN NOT NULL DEFAULT true, amount_f NUMERIC NOT NULL, token_id INT NOT NULL REFERENCES token (token_id), amount_usd NUMERIC, -- Value of the amount in USD at the moment the tx was inserted in the DB @@ -159,7 +168,7 @@ CREATE TABLE tx ( to_forge_l1_txs_num BIGINT, user_origin BOOLEAN, load_amount BYTEA, - effective_load_amount BYTEA, + load_amount_success BOOLEAN NOT NULL DEFAULT true, load_amount_f NUMERIC, load_amount_usd NUMERIC, -- L2 diff --git a/synchronizer/synchronizer.go b/synchronizer/synchronizer.go index 7a56bfe..ea3ee94 100644 --- a/synchronizer/synchronizer.go +++ b/synchronizer/synchronizer.go @@ -750,7 +750,7 @@ func (s *Synchronizer) rollupSync(ethBlock *common.Block) (*common.RollupData, e // that stateDB can process them. // First try to find them in HistoryDB. - l1UserTxs, err = s.historyDB.GetL1UserTxs(nextForgeL1TxsNum) + l1UserTxs, err = s.historyDB.GetUnforgedL1UserTxs(nextForgeL1TxsNum) if err != nil { return nil, tracerr.Wrap(err) } diff --git a/synchronizer/synchronizer_test.go b/synchronizer/synchronizer_test.go index c37cb8e..6273a6e 100644 --- a/synchronizer/synchronizer_test.go +++ b/synchronizer/synchronizer_test.go @@ -101,6 +101,8 @@ func checkSyncBlock(t *testing.T, s *Synchronizer, blockNum int, block, syncBloc break } } + tx.EffectiveAmount = tx.Amount + tx.EffectiveLoadAmount = tx.LoadAmount assert.Equal(t, &tx, dbTx) //nolint:gosec } @@ -457,7 +459,6 @@ func TestSync(t *testing.T) { assert.Equal(t, int64(2), stats.Sync.LastBlock.Num) checkSyncBlock(t, s, 2, &blocks[0], syncBlock) - // Block 3 syncBlock, discards, err = s.Sync2(ctx, nil)