From c5e3fdad75e62b64fbabd56c58dc3c95817e36b4 Mon Sep 17 00:00:00 2001 From: Arnau B Date: Thu, 7 Jan 2021 15:31:37 +0100 Subject: [PATCH] Fix repeated items when coordinator is updated --- api/api_test.go | 20 +++++++++++--- api/coordinator_test.go | 27 ++++++++++++++----- api/slots_test.go | 8 +++++- db/historydb/historydb.go | 56 ++++++++++++++++++++++++++++----------- 4 files changed, 85 insertions(+), 26 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index 6a2c7c0..365cb78 100644 --- a/api/api_test.go +++ b/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 } diff --git a/api/coordinator_test.go b/api/coordinator_test.go index 30e3b8a..6bc66a7 100644 --- a/api/coordinator_test.go +++ b/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 } diff --git a/api/slots_test.go b/api/slots_test.go index 0a47e66..72885ed 100644 --- a/api/slots_test.go +++ b/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) diff --git a/db/historydb/historydb.go b/db/historydb/historydb.go index 5d97fc6..d9a0d69 100644 --- a/db/historydb/historydb.go +++ b/db/historydb/historydb.go @@ -379,8 +379,12 @@ func (hdb *HistoryDB) GetBestBidAPI(slotNum *int64) (BidAPI, error) { bid := &BidAPI{} 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 + FROM bid INNER JOIN block ON bid.eth_block_num = block.eth_block_num + 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) @@ -396,11 +400,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) @@ -415,14 +423,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) @@ -455,15 +468,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 @@ -477,15 +495,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 { @@ -1637,7 +1655,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) } @@ -1648,9 +1670,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 {