Skip to content

Commit

Permalink
Remove milestone no-ack fetching
Browse files Browse the repository at this point in the history
  • Loading branch information
avalkov committed Feb 28, 2025
1 parent 7c8d7a8 commit 5f49cbc
Show file tree
Hide file tree
Showing 12 changed files with 14 additions and 324 deletions.
2 changes: 0 additions & 2 deletions consensus/bor/heimdall.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,5 @@ type IHeimdallClient interface {
FetchCheckpointCount(ctx context.Context) (int64, error)
FetchMilestone(ctx context.Context) (*milestone.Milestone, error)
FetchMilestoneCount(ctx context.Context) (int64, error)
FetchNoAckMilestone(ctx context.Context, milestoneID string) error // Fetch the bool value whether milestone corresponding to the given id failed in the Heimdall
FetchLastNoAckMilestone(ctx context.Context) (string, error) // Fetch latest failed milestone id
Close()
}
50 changes: 0 additions & 50 deletions consensus/bor/heimdall/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ const (
fetchMilestone = "/milestone/latest"
fetchMilestoneCount = "/milestone/count"

fetchLastNoAckMilestone = "/milestone/last-no-ack"
fetchNoAckMilestone = "/milestone/no-ack/%s"

fetchSpanFormat = "bor/span/%d"
)

Expand Down Expand Up @@ -229,44 +226,6 @@ func (h *HeimdallClient) FetchMilestoneCount(ctx context.Context) (int64, error)
return response.Count, nil
}

// FetchLastNoAckMilestone fetches the last no-ack-milestone from heimdall
func (h *HeimdallClient) FetchLastNoAckMilestone(ctx context.Context) (string, error) {
url, err := lastNoAckMilestoneURL(h.urlString)
if err != nil {
return "", err
}

ctx = withRequestType(ctx, milestoneLastNoAckRequest)

response, err := FetchWithRetry[milestone.MilestoneLastNoAckResponse](ctx, h.client, url, h.closeCh)
if err != nil {
return "", err
}

return response.Result, nil
}

// FetchNoAckMilestone fetches the last no-ack-milestone from heimdall
func (h *HeimdallClient) FetchNoAckMilestone(ctx context.Context, milestoneID string) error {
url, err := noAckMilestoneURL(h.urlString, milestoneID)
if err != nil {
return err
}

ctx = withRequestType(ctx, milestoneNoAckRequest)

response, err := FetchWithRetry[milestone.MilestoneNoAckResponse](ctx, h.client, url, h.closeCh)
if err != nil {
return err
}

if !response.Result {
return fmt.Errorf("%w: milestoneID %q", ErrNotInRejectedList, milestoneID)
}

return nil
}

// FetchWithRetry returns data from heimdall with retry
func FetchWithRetry[T any](ctx context.Context, client http.Client, url *url.URL, closeCh chan struct{}) (*T, error) {
// request data once
Expand Down Expand Up @@ -417,15 +376,6 @@ func milestoneCountURL(urlString string) (*url.URL, error) {
return makeURL(urlString, fetchMilestoneCount, "")
}

func lastNoAckMilestoneURL(urlString string) (*url.URL, error) {
return makeURL(urlString, fetchLastNoAckMilestone, "")
}

func noAckMilestoneURL(urlString string, id string) (*url.URL, error) {
url := fmt.Sprintf(fetchNoAckMilestone, id)
return makeURL(urlString, url, "")
}

func makeURL(urlString, rawPath, rawQuery string) (*url.URL, error) {
u, err := url.Parse(urlString)
if err != nil {
Expand Down
28 changes: 2 additions & 26 deletions consensus/bor/heimdall/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import (
// requests to the mock heimdal server for specific functions. Add more handlers
// according to requirements.
type HttpHandlerFake struct {
handleFetchCheckpoint http.HandlerFunc
handleFetchMilestone http.HandlerFunc
handleFetchNoAckMilestone http.HandlerFunc
handleFetchLastNoAckMilestone http.HandlerFunc
handleFetchCheckpoint http.HandlerFunc
handleFetchMilestone http.HandlerFunc
}

func (h *HttpHandlerFake) GetCheckpointHandler() http.HandlerFunc {
Expand All @@ -41,18 +39,6 @@ func (h *HttpHandlerFake) GetMilestoneHandler() http.HandlerFunc {
}
}

func (h *HttpHandlerFake) GetNoAckMilestoneHandler() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
h.handleFetchNoAckMilestone.ServeHTTP(w, r)
}
}

func (h *HttpHandlerFake) GetLastNoAckMilestoneHandler() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
h.handleFetchLastNoAckMilestone.ServeHTTP(w, r)
}
}

func CreateMockHeimdallServer(wg *sync.WaitGroup, port int, listener net.Listener, handler *HttpHandlerFake) (*http.Server, error) {
// Create a new server mux
mux := http.NewServeMux()
Expand All @@ -67,16 +53,6 @@ func CreateMockHeimdallServer(wg *sync.WaitGroup, port int, listener net.Listene
handler.GetMilestoneHandler()(w, r)
})

// Create a route for fetching milestone
mux.HandleFunc("/milestone/noAck/{id}", func(w http.ResponseWriter, r *http.Request) {
handler.GetNoAckMilestoneHandler()(w, r)
})

// Create a route for fetching milestone
mux.HandleFunc("/milestone/lastNoAck", func(w http.ResponseWriter, r *http.Request) {
handler.GetLastNoAckMilestoneHandler()(w, r)
})

// Add other routes as per requirement

// Create the server with given port and mux
Expand Down
30 changes: 7 additions & 23 deletions consensus/bor/heimdall/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ type (
)

const (
stateSyncRequest requestType = "state-sync"
spanRequest requestType = "span"
checkpointRequest requestType = "checkpoint"
checkpointCountRequest requestType = "checkpoint-count"
milestoneRequest requestType = "milestone"
milestoneCountRequest requestType = "milestone-count"
milestoneNoAckRequest requestType = "milestone-no-ack"
milestoneLastNoAckRequest requestType = "milestone-last-no-ack"
milestoneIDRequest requestType = "milestone-id"
stateSyncRequest requestType = "state-sync"
spanRequest requestType = "span"
checkpointRequest requestType = "checkpoint"
checkpointCountRequest requestType = "checkpoint-count"
milestoneRequest requestType = "milestone"
milestoneCountRequest requestType = "milestone-count"
milestoneIDRequest requestType = "milestone-id"
)

func withRequestType(ctx context.Context, reqType requestType) context.Context {
Expand Down Expand Up @@ -82,20 +80,6 @@ var (
},
timer: metrics.NewRegisteredTimer("client/requests/milestonecount/duration", nil),
},
milestoneNoAckRequest: {
request: map[bool]metrics.Meter{
true: metrics.NewRegisteredMeter("client/requests/milestonenoack/valid", nil),
false: metrics.NewRegisteredMeter("client/requests/milestonenoack/invalid", nil),
},
timer: metrics.NewRegisteredTimer("client/requests/milestonenoack/duration", nil),
},
milestoneLastNoAckRequest: {
request: map[bool]metrics.Meter{
true: metrics.NewRegisteredMeter("client/requests/milestonelastnoack/valid", nil),
false: metrics.NewRegisteredMeter("client/requests/milestonelastnoack/invalid", nil),
},
timer: metrics.NewRegisteredTimer("client/requests/milestonelastnoack/duration", nil),
},
milestoneIDRequest: {
request: map[bool]metrics.Meter{
true: metrics.NewRegisteredMeter("client/requests/milestoneid/valid", nil),
Expand Down
8 changes: 0 additions & 8 deletions consensus/bor/heimdall/milestone/milestone.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,3 @@ type MilestoneResponse struct {
type MilestoneCountResponse struct {
Count int64 `json:"count"`
}

type MilestoneLastNoAckResponse struct {
Result string `json:"result"`
}

type MilestoneNoAckResponse struct {
Result bool `json:"result"`
}
30 changes: 0 additions & 30 deletions consensus/bor/heimdallapp/milestone.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package heimdallapp

import (
"context"
"fmt"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/milestone"
Expand Down Expand Up @@ -39,35 +38,6 @@ func (h *HeimdallAppClient) FetchMilestone(_ context.Context) (*milestone.Milest
return milestone, nil
}

func (h *HeimdallAppClient) FetchNoAckMilestone(_ context.Context, milestoneID string) error {
log.Debug("Fetching No Ack Milestone By MilestoneID", "MilestoneID", milestoneID)

res, err := h.hApp.MilestoneKeeper.HasNoAckMilestone(h.NewContext(), milestoneID)
if err != nil {
return err
}

if res {
log.Info("Fetched No Ack By MilestoneID", "MilestoneID", milestoneID)
return nil
}

return fmt.Errorf("still no-ack milestone exist corresponding to milestoneID: %v", milestoneID)
}

func (h *HeimdallAppClient) FetchLastNoAckMilestone(_ context.Context) (string, error) {
log.Debug("Fetching Latest No Ack Milestone ID")

res, err := h.hApp.MilestoneKeeper.GetLastNoAckMilestone(h.NewContext())
if err != nil {
return "", err
}

log.Debug("Fetched Latest No Ack Milestone ID", "res", res)

return res, nil
}

func toBorMilestone(hdMilestone *milestoneTypes.Milestone) *milestone.Milestone {
return &milestone.Milestone{
Proposer: common.HexToAddress(hdMilestone.Proposer),
Expand Down
37 changes: 0 additions & 37 deletions consensus/bor/heimdallgrpc/milestone.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@ package heimdallgrpc

import (
"context"
"fmt"

"github.com/ethereum/go-ethereum/consensus/bor/heimdall"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/milestone"
"github.com/ethereum/go-ethereum/log"

proto "github.com/maticnetwork/polyproto/heimdall"
protoutils "github.com/maticnetwork/polyproto/utils"
)

Expand Down Expand Up @@ -46,37 +43,3 @@ func (h *HeimdallGRPCClient) FetchMilestone(ctx context.Context) (*milestone.Mil

return milestone, nil
}

func (h *HeimdallGRPCClient) FetchLastNoAckMilestone(ctx context.Context) (string, error) {
log.Debug("Fetching latest no ack milestone Id")

res, err := h.client.FetchLastNoAckMilestone(ctx, nil)
if err != nil {
return "", err
}

log.Debug("Fetched last no-ack milestone", "res", res.Result.Result)

return res.Result.Result, nil
}

func (h *HeimdallGRPCClient) FetchNoAckMilestone(ctx context.Context, milestoneID string) error {
req := &proto.FetchMilestoneNoAckRequest{
MilestoneID: milestoneID,
}

log.Debug("Fetching no ack milestone", "milestoneID", milestoneID)

res, err := h.client.FetchNoAckMilestone(ctx, req)
if err != nil {
return err
}

if !res.Result.Result {
return fmt.Errorf("%w: milestoneID %q", heimdall.ErrNotInRejectedList, milestoneID)
}

log.Debug("Fetched no ack milestone", "milestoneID", milestoneID)

return nil
}
53 changes: 1 addition & 52 deletions eth/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,8 +635,6 @@ func (s *Ethereum) Start() error {

go s.startCheckpointWhitelistService()
go s.startMilestoneWhitelistService()
go s.startNoAckMilestoneService()
go s.startNoAckMilestoneByIDService()

return nil
}
Expand All @@ -647,8 +645,7 @@ var (
)

const (
whitelistTimeout = 30 * time.Second
noAckMilestoneTimeout = 4 * time.Second
whitelistTimeout = 30 * time.Second
)

// StartCheckpointWhitelistService starts the goroutine to fetch checkpoints and update the
Expand All @@ -673,24 +670,6 @@ func (s *Ethereum) startMilestoneWhitelistService() {
s.retryHeimdallHandler(s.handleMilestone, tickerDuration, whitelistTimeout, fnName)
}

func (s *Ethereum) startNoAckMilestoneService() {
const (
tickerDuration = 1 * time.Second
fnName = "no-ack-milestone service"
)

s.retryHeimdallHandler(s.handleNoAckMilestone, tickerDuration, noAckMilestoneTimeout, fnName)
}

func (s *Ethereum) startNoAckMilestoneByIDService() {
const (
tickerDuration = 1 * time.Minute
fnName = "no-ack-milestone-by-id service"
)

s.retryHeimdallHandler(s.handleNoAckMilestoneByID, tickerDuration, noAckMilestoneTimeout, fnName)
}

func (s *Ethereum) retryHeimdallHandler(fn heimdallHandler, tickerDuration time.Duration, timeout time.Duration, fnName string) {
retryHeimdallHandler(fn, tickerDuration, timeout, fnName, s.closeCh, s.getHandler)
}
Expand Down Expand Up @@ -787,36 +766,6 @@ func (s *Ethereum) handleMilestone(ctx context.Context, ethHandler *ethHandler,
return nil
}

func (s *Ethereum) handleNoAckMilestone(ctx context.Context, ethHandler *ethHandler, bor *bor.Bor) error {
milestoneID, err := ethHandler.fetchNoAckMilestone(ctx, bor)

if errors.Is(err, heimdall.ErrServiceUnavailable) {
return nil
}

if err != nil {
return err
}

ethHandler.downloader.RemoveMilestoneID(milestoneID)

return nil
}

func (s *Ethereum) handleNoAckMilestoneByID(ctx context.Context, ethHandler *ethHandler, bor *bor.Bor) error {
milestoneIDs := ethHandler.downloader.GetMilestoneIDsList()

for _, milestoneID := range milestoneIDs {
// todo: check if we can ignore the error
err := ethHandler.fetchNoAckMilestoneByID(ctx, bor, milestoneID)
if err == nil {
ethHandler.downloader.RemoveMilestoneID(milestoneID)
}
}

return nil
}

func (s *Ethereum) getHandler() (*ethHandler, *bor.Bor, error) {
ethHandler := (*ethHandler)(s.handler)

Expand Down
17 changes: 0 additions & 17 deletions eth/handler_bor.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/consensus/bor"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall"
"github.com/ethereum/go-ethereum/log"
)

Expand Down Expand Up @@ -93,22 +92,6 @@ func (h *ethHandler) fetchWhitelistMilestone(ctx context.Context, bor *bor.Bor,
return num, hash, err
}

func (h *ethHandler) fetchNoAckMilestone(ctx context.Context, bor *bor.Bor) (string, error) {
milestoneID, err := bor.HeimdallClient.FetchLastNoAckMilestone(ctx)
err = reportCommonErrors("latest no-ack milestone", err, nil)

return milestoneID, err
}

func (h *ethHandler) fetchNoAckMilestoneByID(ctx context.Context, bor *bor.Bor, milestoneID string) error {
err := bor.HeimdallClient.FetchNoAckMilestone(ctx, milestoneID)
if errors.Is(err, heimdall.ErrNotInRejectedList) {
log.Debug("MilestoneID not in rejected list", "milestoneID", milestoneID)
}
err = reportCommonErrors("no-ack milestone by ID", err, nil, "milestoneID", milestoneID)
return err
}

// reportCommonErrors reports common errors which can occur while fetching data from heimdall. It also
// returns back the wrapped erorr if required to the caller.
func reportCommonErrors(msg string, err error, wrapError error, ctx ...interface{}) error {
Expand Down
Loading

0 comments on commit 5f49cbc

Please sign in to comment.