Merge pull request #326 from hermeznetwork/feature/effective-amount-update

Feature/effective amount update

api/swagger.yml

@@ -60,7 +60,7 @@ externalDocs:
   url: 'https://hermez.io'
 servers:
   - description: Hosted mock up
-    url: http://167.71.59.190:4010
+    url: https://apimock.hermez.network
   - description: Localhost mock Up
     url: http://localhost:4010
 tags:
@@ -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`.

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)
 		}

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)
 		}

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 {

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"`

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

synchronizer/synchronizer.go

@@ -751,7 +751,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)
 			}

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)