Merge pull request #468 from hermeznetwork/fix/repeated-bids

Fix repeated items when coordinator is updated

api/api_test.go

@@ -353,6 +353,14 @@ func TestMain(m *testing.M) {
 	// Generate Coordinators and add them to HistoryDB
 	const nCoords = 10
 	commonCoords := test.GenCoordinators(nCoords, commonBlocks)
+	// Update one coordinator to test behaviour when bidder address is repeated
+	updatedCoordBlock := commonCoords[len(commonCoords)-1].EthBlockNum
+	commonCoords = append(commonCoords, common.Coordinator{
+		Bidder:      commonCoords[0].Bidder,
+		Forger:      commonCoords[0].Forger,
+		EthBlockNum: updatedCoordBlock,
+		URL:         commonCoords[0].URL + ".new",
+	})
 	if err := api.h.AddCoordinators(commonCoords); err != nil {
 		panic(err)
 	}
@@ -473,7 +481,7 @@ func TestMain(m *testing.M) {
 	nonBootForger := historydb.CoordinatorAPI{
 		Bidder: commonCoords[0].Bidder,
 		Forger: commonCoords[0].Forger,
-		URL:    commonCoords[0].URL,
+		URL:    commonCoords[0].URL + ".new",
 	}
 	// Slot 4
 	nextForgers[3].Coordinator = nonBootForger
@@ -762,10 +770,16 @@ func getBlockByNum(ethBlockNum int64, blocks []common.Block) common.Block {
 }
 
 func getCoordinatorByBidder(bidder ethCommon.Address, coordinators []historydb.CoordinatorAPI) historydb.CoordinatorAPI {
+	var coordLastUpdate historydb.CoordinatorAPI
+	found := false
 	for _, c := range coordinators {
 		if c.Bidder == bidder {
-			return c
+			coordLastUpdate = c
+			found = true
 		}
 	}
-	panic("coordinator not found")
+	if !found {
+		panic("coordinator not found")
+	}
+	return coordLastUpdate
 }

api/coordinator_test.go

@@ -31,12 +31,27 @@ func (t testCoordinatorsResponse) New() Pendinger { return &testCoordinatorsResp
 func genTestCoordinators(coordinators []common.Coordinator) []historydb.CoordinatorAPI {
 	testCoords := []historydb.CoordinatorAPI{}
 	for i := 0; i < len(coordinators); i++ {
-		testCoords = append(testCoords, historydb.CoordinatorAPI{
-			Bidder:      coordinators[i].Bidder,
-			Forger:      coordinators[i].Forger,
-			EthBlockNum: coordinators[i].EthBlockNum,
-			URL:         coordinators[i].URL,
-		})
+		alreadyRegistered := false
+		for j := 0; j < len(testCoords); j++ {
+			// coordinator already registered
+			if coordinators[i].Bidder == testCoords[j].Bidder {
+				alreadyRegistered = true
+				if coordinators[i].EthBlockNum > testCoords[j].EthBlockNum {
+					// This occurrence is more updated, delete current and append new
+					testCoords = append(testCoords[:j], testCoords[j+1:]...)
+					alreadyRegistered = false
+				}
+				break
+			}
+		}
+		if !alreadyRegistered {
+			testCoords = append(testCoords, historydb.CoordinatorAPI{
+				Bidder:      coordinators[i].Bidder,
+				Forger:      coordinators[i].Forger,
+				EthBlockNum: coordinators[i].EthBlockNum,
+				URL:         coordinators[i].URL,
+			})
+		}
 	}
 	return testCoords
 }

api/slots_test.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 	"testing"
 
+	ethCommon "github.com/ethereum/go-ethereum/common"
 	"github.com/hermeznetwork/hermez-node/common"
 	"github.com/hermeznetwork/hermez-node/db/historydb"
 	"github.com/mitchellh/copystructure"
@@ -148,7 +149,12 @@ func TestGetSlots(t *testing.T) {
 	// maxSlotNum & wonByEthereumAddress
 	fetchedSlots = []testSlot{}
 	limit = 1
-	bidderAddr := tc.coordinators[0].Bidder
+	var bidderAddr ethCommon.Address
+	for i := 0; i < len(tc.slots); i++ {
+		if tc.slots[i].WinnerBid != nil {
+			bidderAddr = tc.slots[i].WinnerBid.Bidder
+		}
+	}
 	path = fmt.Sprintf("%s?maxSlotNum=%d&wonByEthereumAddress=%s&limit=%d", endpoint, maxSlotNum, bidderAddr.String(), limit)
 	err = doGoodReqPaginated(path, historydb.OrderAsc, &testSlotsResponse{}, appendIter)
 	assert.NoError(t, err)

db/historydb/historydb.go

@@ -381,7 +381,11 @@ func (hdb *HistoryDB) GetBestBidAPI(slotNum *int64) (BidAPI, error) {
 	err := meddler.QueryRow(
 		hdb.db, bid, `SELECT bid.*, block.timestamp, coordinator.forger_addr, coordinator.url 
 		FROM bid INNER JOIN block ON bid.eth_block_num = block.eth_block_num
-		INNER JOIN coordinator ON bid.bidder_addr = coordinator.bidder_addr 
+		INNER JOIN (
+			SELECT bidder_addr, MAX(item_id) AS item_id FROM coordinator
+			GROUP BY bidder_addr
+		) c ON bid.bidder_addr = c.bidder_addr 
+		INNER JOIN coordinator ON c.item_id = coordinator.item_id 
 		WHERE slot_num = $1 ORDER BY item_id DESC LIMIT 1;`, slotNum,
 	)
 	return *bid, tracerr.Wrap(err)
@@ -397,11 +401,15 @@ func (hdb *HistoryDB) GetBestBidCoordinator(slotNum int64) (*common.BidCoordinat
 			FROM auction_vars
 			WHERE default_slot_set_bid_slot_num <= $1
 			ORDER BY eth_block_num DESC LIMIT 1
-			),
+		),
 		bid.slot_num, bid.bid_value, bid.bidder_addr,
 		coordinator.forger_addr, coordinator.url
 		FROM bid
-		INNER JOIN coordinator ON bid.bidder_addr = coordinator.bidder_addr
+		INNER JOIN (
+			SELECT bidder_addr, MAX(item_id) AS item_id FROM coordinator
+			GROUP BY bidder_addr
+		) c ON bid.bidder_addr = c.bidder_addr 
+		INNER JOIN coordinator ON c.item_id = coordinator.item_id
 		WHERE bid.slot_num = $1 ORDER BY bid.item_id DESC LIMIT 1;`,
 		slotNum)
 
@@ -416,14 +424,19 @@ func (hdb *HistoryDB) GetBestBidsAPI(
 ) ([]BidAPI, uint64, error) {
 	var query string
 	var args []interface{}
+	// JOIN the best bid of each slot with the latest update of each coordinator
 	queryStr := `SELECT b.*, block.timestamp, coordinator.forger_addr, coordinator.url, 
 	COUNT(*) OVER() AS total_items FROM (
 	   SELECT slot_num, MAX(item_id) as maxitem 
 	   FROM bid GROUP BY slot_num
-	   )
+	)
 	AS x INNER JOIN bid AS b ON b.item_id = x.maxitem
 	INNER JOIN block ON b.eth_block_num = block.eth_block_num
-	INNER JOIN coordinator ON b.bidder_addr = coordinator.bidder_addr 
+	INNER JOIN (
+		SELECT bidder_addr, MAX(item_id) AS item_id FROM coordinator
+		GROUP BY bidder_addr
+	) c ON b.bidder_addr = c.bidder_addr 
+	INNER JOIN coordinator ON c.item_id = coordinator.item_id 
 	WHERE (b.slot_num >= ? AND b.slot_num <= ?)`
 	args = append(args, minSlotNum)
 	args = append(args, maxSlotNum)
@@ -456,15 +469,20 @@ func (hdb *HistoryDB) GetBestBidsAPI(
 
 // GetBidsAPI return the bids applying the given filters
 func (hdb *HistoryDB) GetBidsAPI(
-	slotNum *int64, forgerAddr *ethCommon.Address,
+	slotNum *int64, bidderAddr *ethCommon.Address,
 	fromItem, limit *uint, order string,
 ) ([]BidAPI, uint64, error) {
 	var query string
 	var args []interface{}
-	queryStr := `SELECT bid.*, block.timestamp, coordinator.forger_addr, coordinator.url, 
+	// JOIN each bid with the latest update of each coordinator
+	queryStr := `SELECT bid.*, block.timestamp, coord.forger_addr, coord.url, 
 	COUNT(*) OVER() AS total_items
 	FROM bid INNER JOIN block ON bid.eth_block_num = block.eth_block_num 
-	INNER JOIN coordinator ON bid.bidder_addr = coordinator.bidder_addr `
+	INNER JOIN (
+		SELECT bidder_addr, MAX(item_id) AS item_id FROM coordinator
+		GROUP BY bidder_addr
+	) c ON bid.bidder_addr = c.bidder_addr 
+	INNER JOIN coordinator coord ON c.item_id = coord.item_id `
 	// Apply filters
 	nextIsAnd := false
 	// slotNum filter
@@ -478,15 +496,15 @@ func (hdb *HistoryDB) GetBidsAPI(
 		args = append(args, slotNum)
 		nextIsAnd = true
 	}
-	// slotNum filter
-	if forgerAddr != nil {
+	// bidder filter
+	if bidderAddr != nil {
 		if nextIsAnd {
 			queryStr += "AND "
 		} else {
 			queryStr += "WHERE "
 		}
 		queryStr += "bid.bidder_addr = ? "
-		args = append(args, forgerAddr)
+		args = append(args, bidderAddr)
 		nextIsAnd = true
 	}
 	if fromItem != nil {
@@ -1647,7 +1665,11 @@ func (hdb *HistoryDB) AddBlockSCData(blockData *common.BlockData) (err error) {
 // GetCoordinatorAPI returns a coordinator by its bidderAddr
 func (hdb *HistoryDB) GetCoordinatorAPI(bidderAddr ethCommon.Address) (*CoordinatorAPI, error) {
 	coordinator := &CoordinatorAPI{}
-	err := meddler.QueryRow(hdb.db, coordinator, "SELECT * FROM coordinator WHERE bidder_addr = $1;", bidderAddr)
+	err := meddler.QueryRow(
+		hdb.db, coordinator,
+		"SELECT * FROM coordinator WHERE bidder_addr = $1 ORDER BY item_id DESC LIMIT 1;",
+		bidderAddr,
+	)
 	return coordinator, tracerr.Wrap(err)
 }
 
@@ -1658,9 +1680,11 @@ func (hdb *HistoryDB) GetCoordinatorsAPI(
 ) ([]CoordinatorAPI, uint64, error) {
 	var query string
 	var args []interface{}
-	queryStr := `SELECT coordinator.*, 
-	COUNT(*) OVER() AS total_items
-	FROM coordinator `
+	queryStr := `SELECT coordinator.*, COUNT(*) OVER() AS total_items
+	FROM coordinator INNER JOIN (
+		SELECT MAX(item_id) AS item_id FROM coordinator
+		GROUP BY bidder_addr
+	) c ON coordinator.item_id = c.item_id `
 	// Apply filters
 	nextIsAnd := false
 	if bidderAddr != nil {