diff --git a/consensus/bor/bor.go b/consensus/bor/bor.go index 7adebb9e4c4c56ae136dea5a510fd554211308e1..d3fde78751411a44983cbcf07633736e39b751df 100644 --- a/consensus/bor/bor.go +++ b/consensus/bor/bor.go @@ -12,7 +12,6 @@ import ( "math/big" "sort" - "strconv" "strings" "sync" "time" @@ -967,7 +966,7 @@ func (c *Bor) fetchAndCommitSpan( header *types.Header, chain core.ChainContext, ) error { - response, err := c.HeimdallClient.FetchWithRetry("bor", "span", strconv.FormatUint(newSpanID, 10)) + response, err := c.HeimdallClient.FetchWithRetry(fmt.Sprintf("bor/span/%d", newSpanID), "") if err != nil { return err @@ -1092,9 +1091,9 @@ func (c *Bor) CommitStates( page := 1 eventRecords := make([]*EventRecordWithTime, 0) for { - query := fmt.Sprintf("clerk/event-record/list?from-time=%d&to-time=%d&page=%d&limit=%d", from.Unix(), to.Unix(), page, stateFetchLimit) - log.Info("Fetching state events", "query", query) - response, err := c.HeimdallClient.FetchWithRetry(query) + queryParams := fmt.Sprintf("from-time=%d&to-time=%d&page=%d&limit=%d", from.Unix(), to.Unix(), page, stateFetchLimit) + log.Info("Fetching state sync events", "queryParams", queryParams) + response, err := c.HeimdallClient.FetchWithRetry("clerk/event-record/list", queryParams) if err != nil { return err } @@ -1119,7 +1118,7 @@ func (c *Bor) CommitStates( for _, eventRecord := range eventRecords { // if chanId doesn't match with the record, simply ignore it if eventRecord.ChainID != chainID { - log.Error(fmt.Sprintln("Chain Id in received event %s and bor chain Id %s, don't match", eventRecord, chainID)) + log.Error(fmt.Sprintf("Chain Id in received event %s and bor chain Id %s, don't match", eventRecord, chainID)) continue } // validateEventRecord checks whether an event lies in the specified time range diff --git a/consensus/bor/bor_test/bor_test.go b/consensus/bor/bor_test/bor_test.go index 8b2fde3be4a383f6db0b0328fe0086bc7d17e749..d9391f1787b95445cd31d0d3d570a919f4c7fc72 100644 --- a/consensus/bor/bor_test/bor_test.go +++ b/consensus/bor/bor_test/bor_test.go @@ -20,6 +20,12 @@ import ( "github.com/maticnetwork/bor/mocks" ) +var ( + spanPath = "bor/span/1" + clerkPath = "clerk/event-record/list" + clerkQueryParams = "from-time=%d&to-time=%d&page=%d&limit=50" +) + func TestInsertingSpanSizeBlocks(t *testing.T) { init := buildEthereumInstance(t, rawdb.NewMemoryDatabase()) chain := init.ethereum.BlockChain() @@ -42,10 +48,9 @@ func TestInsertingSpanSizeBlocks(t *testing.T) { } } - assert.True(t, h.AssertCalled(t, "FetchWithRetry", "bor", "span", "1")) - query := fmt.Sprintf("clerk/event-record/list?from-time=%d&to-time=%d&page=1&limit=50", 1, to) - assert.True(t, h.AssertCalled(t, "FetchWithRetry", query)) - validators, err := _bor.GetCurrentValidators(sprintSize, spanSize) // new span starts at 256 + assert.True(t, h.AssertCalled(t, "FetchWithRetry", spanPath, "")) + assert.True(t, h.AssertCalled(t, "FetchWithRetry", clerkPath, fmt.Sprintf(clerkQueryParams, 1, to, 1))) + validators, err := _bor.GetCurrentValidators(sprintSize, spanSize) // check validator set at the first block of new span if err != nil { t.Fatalf("%s", err) } @@ -77,7 +82,7 @@ func TestFetchStateSyncEvents(t *testing.T) { // B.1 Mock /bor/span/1 res, _ := loadSpanFromFile(t) h := &mocks.IHeimdallClient{} - h.On("FetchWithRetry", "bor", "span", "1").Return(res, nil) + h.On("FetchWithRetry", spanPath, "").Return(res, nil) // B.2 Mock State Sync events // read heimdall api response from file @@ -99,20 +104,20 @@ func TestFetchStateSyncEvents(t *testing.T) { from := 1 to := int64(block.Header().Time) page := 1 - query1 := fmt.Sprintf("clerk/event-record/list?from-time=%d&to-time=%d&page=%d&limit=50", from, to, page) - h.On("FetchWithRetry", query1).Return(&response, nil) + query1Params := fmt.Sprintf(clerkQueryParams, from, to, page) + h.On("FetchWithRetry", clerkPath, query1Params).Return(&response, nil) page = 2 - query2 := fmt.Sprintf("clerk/event-record/list?from-time=%d&to-time=%d&page=%d&limit=50", from, to, page) - h.On("FetchWithRetry", query2).Return(&bor.ResponseWithHeight{}, nil) + query2Params := fmt.Sprintf(clerkQueryParams, from, to, page) + h.On("FetchWithRetry", clerkPath, query2Params).Return(&bor.ResponseWithHeight{}, nil) _bor.SetHeimdallClient(h) block = buildNextBlock(t, _bor, chain, block, nil, init.genesis.Config.Bor) insertNewBlock(t, chain, block) - assert.True(t, h.AssertCalled(t, "FetchWithRetry", "bor", "span", "1")) - assert.True(t, h.AssertCalled(t, "FetchWithRetry", query1)) - assert.True(t, h.AssertCalled(t, "FetchWithRetry", query2)) + assert.True(t, h.AssertCalled(t, "FetchWithRetry", spanPath, "")) + assert.True(t, h.AssertCalled(t, "FetchWithRetry", clerkPath, query1Params)) + assert.True(t, h.AssertCalled(t, "FetchWithRetry", clerkPath, query2Params)) } func TestOutOfTurnSigning(t *testing.T) { @@ -190,8 +195,8 @@ func TestSignerNotFound(t *testing.T) { func getMockedHeimdallClient(t *testing.T) (*mocks.IHeimdallClient, *bor.HeimdallSpan) { res, heimdallSpan := loadSpanFromFile(t) h := &mocks.IHeimdallClient{} - h.On("FetchWithRetry", "bor", "span", "1").Return(res, nil) - h.On("FetchWithRetry", mock.AnythingOfType("string")).Return(stateSyncEventsPayload(t), nil) + h.On("FetchWithRetry", "bor/span/1", "").Return(res, nil) + h.On("FetchWithRetry", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(stateSyncEventsPayload(t), nil) return h, heimdallSpan } diff --git a/consensus/bor/rest.go b/consensus/bor/rest.go index 91aae237a7bbcfa78deff355510504286e4011aa..14bfb7853cff5224b318607276aa284d5920e4da 100644 --- a/consensus/bor/rest.go +++ b/consensus/bor/rest.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "net/http" "net/url" - "path" "time" "github.com/maticnetwork/bor/log" @@ -20,8 +19,8 @@ type ResponseWithHeight struct { } type IHeimdallClient interface { - Fetch(paths ...string) (*ResponseWithHeight, error) - FetchWithRetry(paths ...string) (*ResponseWithHeight, error) + Fetch(path string, query string) (*ResponseWithHeight, error) + FetchWithRetry(path string, query string) (*ResponseWithHeight, error) } type HeimdallClient struct { @@ -39,33 +38,28 @@ func NewHeimdallClient(urlString string) (*HeimdallClient, error) { return h, nil } -func (h *HeimdallClient) Fetch(paths ...string) (*ResponseWithHeight, error) { +// Fetch fetches response from heimdall +func (h *HeimdallClient) Fetch(rawPath string, rawQuery string) (*ResponseWithHeight, error) { u, err := url.Parse(h.urlString) if err != nil { return nil, err } - for _, e := range paths { - if e != "" { - u.Path = path.Join(u.Path, e) - } - } + u.Path = rawPath + u.RawQuery = rawQuery return h.internalFetch(u) } // FetchWithRetry returns data from heimdall with retry -func (h *HeimdallClient) FetchWithRetry(paths ...string) (*ResponseWithHeight, error) { +func (h *HeimdallClient) FetchWithRetry(rawPath string, rawQuery string) (*ResponseWithHeight, error) { u, err := url.Parse(h.urlString) if err != nil { return nil, err } - for _, e := range paths { - if e != "" { - u.Path = path.Join(u.Path, e) - } - } + u.Path = rawPath + u.RawQuery = rawQuery for { res, err := h.internalFetch(u) diff --git a/mocks/IHeimdallClient.go b/mocks/IHeimdallClient.go index cb09800c746d629e87e0cb52269b3be23be0c7ea..5bc7cb6e6202515f35507a06f905c2311964fa6e 100644 --- a/mocks/IHeimdallClient.go +++ b/mocks/IHeimdallClient.go @@ -12,19 +12,13 @@ type IHeimdallClient struct { mock.Mock } -// Fetch provides a mock function with given fields: paths -func (_m *IHeimdallClient) Fetch(paths ...string) (*bor.ResponseWithHeight, error) { - _va := make([]interface{}, len(paths)) - for _i := range paths { - _va[_i] = paths[_i] - } - var _ca []interface{} - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) +// Fetch provides a mock function with given fields: path, query +func (_m *IHeimdallClient) Fetch(path string, query string) (*bor.ResponseWithHeight, error) { + ret := _m.Called(path, query) var r0 *bor.ResponseWithHeight - if rf, ok := ret.Get(0).(func(...string) *bor.ResponseWithHeight); ok { - r0 = rf(paths...) + if rf, ok := ret.Get(0).(func(string, string) *bor.ResponseWithHeight); ok { + r0 = rf(path, query) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*bor.ResponseWithHeight) @@ -32,8 +26,8 @@ func (_m *IHeimdallClient) Fetch(paths ...string) (*bor.ResponseWithHeight, erro } var r1 error - if rf, ok := ret.Get(1).(func(...string) error); ok { - r1 = rf(paths...) + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(path, query) } else { r1 = ret.Error(1) } @@ -41,19 +35,13 @@ func (_m *IHeimdallClient) Fetch(paths ...string) (*bor.ResponseWithHeight, erro return r0, r1 } -// FetchWithRetry provides a mock function with given fields: paths -func (_m *IHeimdallClient) FetchWithRetry(paths ...string) (*bor.ResponseWithHeight, error) { - _va := make([]interface{}, len(paths)) - for _i := range paths { - _va[_i] = paths[_i] - } - var _ca []interface{} - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) +// FetchWithRetry provides a mock function with given fields: path, query +func (_m *IHeimdallClient) FetchWithRetry(path string, query string) (*bor.ResponseWithHeight, error) { + ret := _m.Called(path, query) var r0 *bor.ResponseWithHeight - if rf, ok := ret.Get(0).(func(...string) *bor.ResponseWithHeight); ok { - r0 = rf(paths...) + if rf, ok := ret.Get(0).(func(string, string) *bor.ResponseWithHeight); ok { + r0 = rf(path, query) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*bor.ResponseWithHeight) @@ -61,8 +49,8 @@ func (_m *IHeimdallClient) FetchWithRetry(paths ...string) (*bor.ResponseWithHei } var r1 error - if rf, ok := ret.Get(1).(func(...string) error); ok { - r1 = rf(paths...) + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(path, query) } else { r1 = ret.Error(1) }