From 810887cd35aa10a590def3db03945a3ea308debe Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 18 Aug 2023 11:56:03 -0500 Subject: [PATCH 01/23] add sequencer coordinator management UI tool --- Dockerfile | 1 + Makefile | 5 +- .../rediscoordinator/redis_coordinator.go | 77 ++++++ .../seq-coordinator-manager.go | 249 ++++++++++++++++++ go.mod | 8 +- go.sum | 26 ++ 6 files changed, 364 insertions(+), 2 deletions(-) create mode 100644 cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go create mode 100644 cmd/seq-coordinator-manager/seq-coordinator-manager.go diff --git a/Dockerfile b/Dockerfile index 367d76d4b1..c1a28760c4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -202,6 +202,7 @@ WORKDIR /home/user COPY --from=node-builder /workspace/target/bin/nitro /usr/local/bin/ COPY --from=node-builder /workspace/target/bin/relay /usr/local/bin/ COPY --from=node-builder /workspace/target/bin/nitro-val /usr/local/bin/ +COPY --from=node-builder /workspace/target/bin/seq-coordinator-manager /usr/local/bin/ COPY --from=machine-versions /workspace/machines /home/user/target/machines USER root RUN export DEBIAN_FRONTEND=noninteractive && \ diff --git a/Makefile b/Makefile index 205025dfe9..1358f961e9 100644 --- a/Makefile +++ b/Makefile @@ -88,7 +88,7 @@ push: lint test-go .make/fmt all: build build-replay-env test-gen-proofs @touch .make/all -build: $(patsubst %,$(output_root)/bin/%, nitro deploy relay daserver datool seq-coordinator-invalidate nitro-val) +build: $(patsubst %,$(output_root)/bin/%, nitro deploy relay daserver datool seq-coordinator-invalidate nitro-val seq-coordinator-manager) @printf $(done) build-node-deps: $(go_source) build-prover-header build-prover-lib build-jit .make/solgen .make/cbrotli-lib @@ -185,6 +185,9 @@ $(output_root)/bin/seq-coordinator-invalidate: $(DEP_PREDICATE) build-node-deps $(output_root)/bin/nitro-val: $(DEP_PREDICATE) build-node-deps go build $(GOLANG_PARAMS) -o $@ "$(CURDIR)/cmd/nitro-val" +$(output_root)/bin/seq-coordinator-manager: $(DEP_PREDICATE) build-node-deps + go build $(GOLANG_PARAMS) -o $@ "$(CURDIR)/cmd/seq-coordinator-manager" + # recompile wasm, but don't change timestamp unless files differ $(replay_wasm): $(DEP_PREDICATE) $(go_source) .make/solgen mkdir -p `dirname $(replay_wasm)` diff --git a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go new file mode 100644 index 0000000000..db3724240e --- /dev/null +++ b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go @@ -0,0 +1,77 @@ +package rediscoordinator + +import ( + "context" + "errors" + "strings" + + "github.com/go-redis/redis/v8" + "github.com/offchainlabs/nitro/util/redisutil" +) + +type RedisCoordinator struct { + Client redis.UniversalClient +} + +func NewRedisCoordinator(redisURL string) (*RedisCoordinator, error) { + redisClient, err := redisutil.RedisClientFromURL(redisURL) + if err != nil { + return nil, err + } + + return &RedisCoordinator{ + Client: redisClient, + }, nil +} + +func (rc *RedisCoordinator) GetPriorities(ctx context.Context) ([]string, map[string]int, error) { + prioritiesMap := make(map[string]int) + prioritiesString, err := rc.Client.Get(ctx, redisutil.PRIORITIES_KEY).Result() + if err != nil { + if errors.Is(err, redis.Nil) { + err = errors.New("sequencer priorities unset") + } + return []string{}, prioritiesMap, err + } + priorities := strings.Split(prioritiesString, ",") + for _, url := range priorities { + prioritiesMap[url]++ + } + return priorities, prioritiesMap, nil +} + +func (rc *RedisCoordinator) GetLivelinessMap(ctx context.Context) (map[string]int, error) { + livelinessMap := make(map[string]int) + livelinessList, _, err := rc.Client.Scan(ctx, 0, redisutil.WANTS_LOCKOUT_KEY_PREFIX+"*", 0).Result() + if err != nil { + return livelinessMap, err + } + for _, elem := range livelinessList { + url := strings.TrimPrefix(elem, redisutil.WANTS_LOCKOUT_KEY_PREFIX) + livelinessMap[url]++ + } + return livelinessMap, nil +} + +func (rc *RedisCoordinator) UpdatePriorities(ctx context.Context, priorities []string) error { + prioritiesString := strings.Join(priorities, ",") + err := rc.Client.Set(ctx, redisutil.PRIORITIES_KEY, prioritiesString, 0).Err() + if err != nil { + if errors.Is(err, redis.Nil) { + err = errors.New("sequencer priorities unset") + } + } + return err +} + +// CurrentChosenSequencer retrieves the current chosen sequencer holding the lock +func (c *RedisCoordinator) CurrentChosenSequencer(ctx context.Context) (string, error) { + current, err := c.Client.Get(ctx, redisutil.CHOSENSEQ_KEY).Result() + if errors.Is(err, redis.Nil) { + return "", nil + } + if err != nil { + return "", err + } + return current, nil +} diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go new file mode 100644 index 0000000000..5844e2e7e4 --- /dev/null +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -0,0 +1,249 @@ +package main + +import ( + "context" + "fmt" + "os" + "strconv" + + "github.com/enescakir/emoji" + "github.com/ethereum/go-ethereum/log" + "github.com/gdamore/tcell/v2" + "github.com/offchainlabs/nitro/cmd/seq-coordinator-manager/rediscoordinator" + "github.com/rivo/tview" +) + +// Tview +var pages = tview.NewPages() +var app = tview.NewApplication() + +// Lists +var prioritySeqList = tview.NewList().ShowSecondaryText(false) +var nonPrioritySeqList = tview.NewList().ShowSecondaryText(false) + +// Forms +var addSeqForm = tview.NewForm() +var priorityForm = tview.NewForm() +var nonPriorityForm = tview.NewForm() + +// Sequencer coordinator managment UI data store +type manager struct { + redisCoordinator *rediscoordinator.RedisCoordinator + prioritiesMap map[string]int + livelinessMap map[string]int + priorityList []string + nonPriorityList []string +} + +func main() { + ctx, cancelFunc := context.WithCancel(context.Background()) + defer cancelFunc() + + args := os.Args[1:] + if len(args) != 1 { + fmt.Fprintf(os.Stderr, "Usage: redis-seq-manager [redis-url]\n") + os.Exit(1) + } + redisURL := args[0] + redisCoordinator, err := rediscoordinator.NewRedisCoordinator(redisURL) + if err != nil { + panic(err) + } + + seqManager := &manager{ + redisCoordinator: redisCoordinator, + prioritiesMap: make(map[string]int), + livelinessMap: make(map[string]int), + } + + seqManager.refreshAllLists(ctx) + seqManager.populateLists(ctx) + + prioritySeqList.SetSelectedFunc(func(index int, name string, second_name string, shortcut rune) { + nonPriorityForm.Clear(true) + + n := len(seqManager.priorityList) + priorities := make([]string, n) + for i := 0; i < n; i++ { + priorities[i] = strconv.Itoa(i) + } + + target := index + priorityForm.Clear(true) + priorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:", 0, 2, false, true) + priorityForm.AddDropDown("Change priority to ->", priorities, index, func(priority string, selection int) { + target = selection + }) + priorityForm.AddButton("Save", func() { + if target != index { + seqManager.updatePriorityList(ctx, index, target) + } + seqManager.populateLists(ctx) + pages.SwitchToPage("Menu") + }) + }) + + nonPrioritySeqList.SetSelectedFunc(func(index int, name string, second_name string, shortcut rune) { + priorityForm.Clear(true) + + n := len(seqManager.priorityList) + priorities := make([]string, n+1) + for i := 0; i < n+1; i++ { + priorities[i] = strconv.Itoa(i) + } + + target := index + nonPriorityForm.Clear(true) + nonPriorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:", 0, 2, false, true) + nonPriorityForm.AddDropDown("Set priority to ->", priorities, index, func(priority string, selection int) { + target = selection + }) + nonPriorityForm.AddButton("Save", func() { + seqManager.priorityList = append(seqManager.priorityList, seqManager.nonPriorityList[index]) + index = len(seqManager.priorityList) - 1 + seqManager.updatePriorityList(ctx, index, target) + nonPriorityForm.Clear(true) + seqManager.populateLists(ctx) + pages.SwitchToPage("Menu") + }) + }) + + // UI design + flex := tview.NewFlex() + priorityHeading := tview.NewTextView(). + SetTextColor(tcell.ColorYellow). + SetText("-----Priority List-----") + nonPriorityHeading := tview.NewTextView(). + SetTextColor(tcell.ColorYellow). + SetText("-----Not in priority list but online-----") + instructions := tview.NewTextView(). + SetTextColor(tcell.ColorYellow). + SetText("(r) to refresh \n(a) to add sequencer\n(q) to quit") + + flex.SetDirection(tview.FlexRow). + AddItem(priorityHeading, 0, 1, false). + AddItem(tview.NewFlex(). + AddItem(prioritySeqList, 0, 2, true). + AddItem(priorityForm, 0, 3, false), 0, 12, false). + AddItem(nonPriorityHeading, 0, 1, false). + AddItem(tview.NewFlex(). + AddItem(nonPrioritySeqList, 0, 2, true). + AddItem(nonPriorityForm, 0, 3, false), 0, 12, false). + AddItem(instructions, 0, 2, false).SetBorder(true) + + flex.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey { + if event.Rune() == 114 { + seqManager.refreshAllLists(ctx) + priorityForm.Clear(true) + nonPriorityForm.Clear(true) + seqManager.populateLists(ctx) + pages.SwitchToPage("Menu") + } else if event.Rune() == 97 { + addSeqForm.Clear(true) + seqManager.addSeqPriorityForm(ctx) + pages.SwitchToPage("Add Sequencer") + } else if event.Rune() == 113 { + app.Stop() + } + return event + }) + + pages.AddPage("Menu", flex, true, true) + pages.AddPage("Add Sequencer", addSeqForm, true, false) + + if err := app.SetRoot(pages, true).EnableMouse(true).Run(); err != nil { + panic(err) + } +} + +// updatePriorityList updates the list by changing the position of seq present at `index` to target +func (sm *manager) updatePriorityList(ctx context.Context, index int, target int) { + for i := index - 1; i >= target; i-- { + sm.priorityList[i], sm.priorityList[i+1] = sm.priorityList[i+1], sm.priorityList[i] + } + for i := index + 1; i <= target; i++ { + sm.priorityList[i], sm.priorityList[i-1] = sm.priorityList[i-1], sm.priorityList[i] + } + err := sm.redisCoordinator.UpdatePriorities(ctx, sm.priorityList) + if err != nil { + log.Warn("Failed to update priority, reverting change", "sequencer", sm.priorityList[target], "err", err) + } + sm.refreshAllLists(ctx) +} + +// populateLists populates seq's in priority list and seq's that are online but not in priority +func (sm *manager) populateLists(ctx context.Context) { + prioritySeqList.Clear() + chosen, err := sm.redisCoordinator.CurrentChosenSequencer(ctx) + if err != nil { + panic(err) + } + for index, seqURL := range sm.priorityList { + sec := "" + if seqURL == chosen { + sec = fmt.Sprintf(" %vchosen", emoji.LeftArrow) + } + status := fmt.Sprintf("%v ", emoji.RedCircle) + if _, ok := sm.livelinessMap[seqURL]; ok { + status = fmt.Sprintf("%v ", emoji.GreenCircle) + } + prioritySeqList.AddItem(status+seqURL+sec, "", rune(48+index), nil).SetSecondaryTextColor(tcell.ColorPurple) + } + + nonPrioritySeqList.Clear() + status := fmt.Sprintf("%v ", emoji.GreenCircle) + for _, seqURL := range sm.nonPriorityList { + nonPrioritySeqList.AddItem(status+seqURL, "", rune(45), nil) + } +} + +// addSeqPriorityForm returns a form with fields to add a new sequencer to priority list +func (sm *manager) addSeqPriorityForm(ctx context.Context) *tview.Form { + URL := "" + addSeqForm.AddInputField("Sequencer URL", "", 0, nil, func(url string) { + URL = url + }) + addSeqForm.AddButton("Cancel", func() { + priorityForm.Clear(true) + sm.populateLists(ctx) + pages.SwitchToPage("Menu") + }) + addSeqForm.AddButton("Add", func() { + // check if url is valid, i.e it doesnt already exist in the priority list + if _, ok := sm.prioritiesMap[URL]; !ok && URL != "" { + sm.priorityList = append(sm.priorityList, URL) + err := sm.redisCoordinator.UpdatePriorities(ctx, sm.priorityList) + if err != nil { + log.Warn("Failed to add sequencer to the priority list", URL) + } + sm.refreshAllLists(ctx) + } + sm.populateLists(ctx) + pages.SwitchToPage("Menu") + }) + return addSeqForm +} + +// refreshAllLists gets the current status of all the lists displayed in the UI +func (sm *manager) refreshAllLists(ctx context.Context) { + sequencerURLList, mapping, err := sm.redisCoordinator.GetPriorities(ctx) + if err != nil { + panic(err) + } + sm.priorityList = sequencerURLList + sm.prioritiesMap = mapping + + mapping, err = sm.redisCoordinator.GetLivelinessMap(ctx) + if err != nil { + panic(err) + } + sm.livelinessMap = mapping + + urlList := []string{} + for url := range sm.livelinessMap { + if _, ok := sm.prioritiesMap[url]; !ok { + urlList = append(urlList, url) + } + } + sm.nonPriorityList = urlList +} diff --git a/go.mod b/go.mod index 5adfd19388..e8fa503196 100644 --- a/go.mod +++ b/go.mod @@ -86,11 +86,14 @@ require ( github.com/dustin/go-humanize v1.0.0 // indirect github.com/elastic/gosigar v0.14.2 // indirect github.com/emirpasic/gods v1.18.1 // indirect + github.com/enescakir/emoji v1.0.0 // indirect github.com/facebookgo/atomicfile v0.0.0-20151019160806-2de1f203e7d5 // indirect github.com/flynn/noise v1.0.0 // indirect github.com/francoispqt/gojay v1.2.13 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/gammazero/deque v0.2.1 // indirect + github.com/gdamore/encoding v1.0.0 // indirect + github.com/gdamore/tcell/v2 v2.6.0 // indirect github.com/getsentry/sentry-go v0.18.0 // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect @@ -188,6 +191,7 @@ require ( github.com/libp2p/go-reuseport v0.2.0 // indirect github.com/libp2p/go-yamux/v4 v4.0.0 // indirect github.com/libp2p/zeroconf/v2 v2.2.0 // indirect + github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/marten-seemann/tcp v0.0.0-20210406111302-dfbc87cc63fd // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/miekg/dns v1.1.50 // indirect @@ -224,6 +228,8 @@ require ( github.com/quic-go/webtransport-go v0.5.2 // indirect github.com/raulk/go-watchdog v1.3.0 // indirect github.com/rhnvrm/simples3 v0.6.1 // indirect + github.com/rivo/tview v0.0.0-20230814110005-ccc2c8119703 // indirect + github.com/rivo/uniseg v0.4.3 // indirect github.com/rogpeppe/go-internal v1.9.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/samber/lo v1.36.0 // indirect @@ -298,7 +304,7 @@ require ( github.com/jackpal/go-nat-pmp v1.0.2 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.17 // indirect - github.com/mattn/go-runewidth v0.0.9 // indirect + github.com/mattn/go-runewidth v0.0.14 // indirect github.com/mitchellh/mapstructure v1.4.2 github.com/mitchellh/pointerstructure v1.2.0 // indirect github.com/olekukonko/tablewriter v0.0.5 // indirect diff --git a/go.sum b/go.sum index 58155db124..5f03dee5b3 100644 --- a/go.sum +++ b/go.sum @@ -310,6 +310,8 @@ github.com/elastic/gosigar v0.14.2 h1:Dg80n8cr90OZ7x+bAax/QjoW/XqTI11RmA79ZwIm9/ github.com/elastic/gosigar v0.14.2/go.mod h1:iXRIGg2tLnu7LBdpqzyQfGDEidKCfWcCMS0WKyPWoMs= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= +github.com/enescakir/emoji v1.0.0 h1:W+HsNql8swfCQFtioDGDHCHri8nudlK1n5p2rHCJoog= +github.com/enescakir/emoji v1.0.0/go.mod h1:Bt1EKuLnKDTYpLALApstIkAjdDrS/8IAgTkKp+WKFD0= github.com/envoyproxy/go-control-plane v0.6.9/go.mod h1:SBwIajubJHhxtWwsL9s8ss4safvEdbitLhGGK48rN6g= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= @@ -348,6 +350,10 @@ github.com/gammazero/deque v0.2.1/go.mod h1:LFroj8x4cMYCukHJDbxFCkT+r9AndaJnFMuZ github.com/gavv/httpexpect v2.0.0+incompatible/go.mod h1:x+9tiU1YnrOvnB725RkpoLv1M62hOWzwo5OXotisrKc= github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff h1:tY80oXqGNY4FhTFhk+o9oFHGINQ/+vhlm8HFzi6znCI= github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff/go.mod h1:x7DCsMOv1taUwEWCzT4cmDeAkigA5/QCwUodaVOe8Ww= +github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= +github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg= +github.com/gdamore/tcell/v2 v2.6.0 h1:OKbluoP9VYmJwZwq/iLb4BxwKcwGthaa1YNBJIyCySg= +github.com/gdamore/tcell/v2 v2.6.0/go.mod h1:be9omFATkdr0D9qewWW3d+MEvl5dha+Etb5y65J2H8Y= github.com/getsentry/sentry-go v0.12.0/go.mod h1:NSap0JBYWzHND8oMbyi0+XZhUalc1TBdRL1M71JZW2c= github.com/getsentry/sentry-go v0.18.0 h1:MtBW5H9QgdcJabtZcuJG80BMOwaBpkRDZkxRkNC1sN0= github.com/getsentry/sentry-go v0.18.0/go.mod h1:Kgon4Mby+FJ7ZWHFUAZgVaIa8sxHtnRJRLTXZr51aKQ= @@ -1140,6 +1146,8 @@ github.com/libp2p/zeroconf/v2 v2.2.0/go.mod h1:fuJqLnUwZTshS3U/bMRJ3+ow/v9oid1n0 github.com/lightstep/lightstep-tracer-common/golang/gogo v0.0.0-20190605223551-bc2310a04743/go.mod h1:qklhhLq1aX+mtWk9cPHPzaBjWImj5ULL6C7HFJtXQMM= github.com/lightstep/lightstep-tracer-go v0.18.1/go.mod h1:jlF1pusYV4pidLvZ+XD0UBX0ZE6WURAspgAczcDHrL4= github.com/lucas-clemente/quic-go v0.19.3/go.mod h1:ADXpNbTQjq1hIzCpB+y/k5iz4n4z4IwqoLb94Kh5Hu8= +github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= +github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm/+2c2E2WMI= github.com/lyft/protoc-gen-validate v0.0.13/go.mod h1:XbGvPuh87YZc5TdIa2/I4pLk0QoUACkjt2znoq26NVQ= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= @@ -1174,6 +1182,8 @@ github.com/mattn/go-isatty v0.0.17/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/ github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= github.com/mattn/go-runewidth v0.0.9 h1:Lm995f3rfxdpd6TSmuVCHVb/QhupuXlYr8sCI/QdE+0= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= +github.com/mattn/go-runewidth v0.0.14 h1:+xnbZSEeDbOIg5/mE6JF0w6n9duR1l3/WmbinWVwUuU= +github.com/mattn/go-runewidth v0.0.14/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= github.com/mattn/goveralls v0.0.2/go.mod h1:8d1ZMHsd7fW6IRPKQh46F2WRpyib5/X4FOpevwGNQEw= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= @@ -1443,6 +1453,11 @@ github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqn github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/rhnvrm/simples3 v0.6.1 h1:H0DJwybR6ryQE+Odi9eqkHuzjYAeJgtGcGtuBwOhsH8= github.com/rhnvrm/simples3 v0.6.1/go.mod h1:Y+3vYm2V7Y4VijFoJHHTrja6OgPrJ2cBti8dPGkC3sA= +github.com/rivo/tview v0.0.0-20230814110005-ccc2c8119703 h1:ZyM/+FYnpbZsFWuCohniM56kRoHRB4r5EuIzXEYkpxo= +github.com/rivo/tview v0.0.0-20230814110005-ccc2c8119703/go.mod h1:nVwGv4MP47T0jvlk7KuTTjjuSmrGO4JF0iaiNt4bufE= +github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= +github.com/rivo/uniseg v0.4.3 h1:utMvzDsuh3suAEnhH0RdHmoPbU648o6CvXxTx4SBMOw= +github.com/rivo/uniseg v0.4.3/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= @@ -1631,6 +1646,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/yuin/gopher-lua v0.0.0-20210529063254-f4c35e4016d9 h1:k/gmLsJDWwWqbLCur2yWnJzwQEKRcAHXo6seXGuSwWw= github.com/yuin/gopher-lua v0.0.0-20210529063254-f4c35e4016d9/go.mod h1:E1AXubJBdNmFERAOucpDIxNzeGfLzg0mYh+UfMWdChA= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= @@ -1774,6 +1790,7 @@ golang.org/x/mod v0.1.1-0.20191107180719-034126e5016b/go.mod h1:QqPTAvyqsEbceGzB golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.9.0 h1:KENHtAZL2y3NLMYZeHY9DW8HW8V+kQyJsY/V9JlKvCs= golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180406214816-61147c48b25b/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1835,6 +1852,7 @@ golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT golang.org/x/net v0.0.0-20210726213435-c6fcb2dbf985/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20210917221730-978cfadd31cf/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211008194852-3b03d305991f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.8.0 h1:Zrh2ngAOFYneWTAIAPethzeaQLuHwhuBkuV6ZiRnUaQ= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -1857,6 +1875,7 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180810173357-98c5dad5d1a0/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -1948,13 +1967,18 @@ golang.org/x/sys v0.0.0-20210910150752-751e447fb3d0/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211007075335-d3039528d8ac/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.6.0 h1:clScbb1cHjoCkyRbWwBEUZ5H/tIFu5TAXIqaZD0Gcjw= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -1966,6 +1990,7 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.8.0 h1:57P1ETyNKtuIjB4SRd15iJxuhj8Gc416Y78H3qgMh68= golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -2035,6 +2060,7 @@ golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.6-0.20210726203631-07bc1bf47fb2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From ef30f3187e85ba9d80b63a7a06c9e68ef4c30539 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 21 Aug 2023 10:25:56 -0500 Subject: [PATCH 02/23] add keyboard-only support with additional functionalities --- .../seq-coordinator-manager.go | 93 +++++++++++++++---- 1 file changed, 75 insertions(+), 18 deletions(-) diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index 5844e2e7e4..f39a810e9b 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -70,17 +70,37 @@ func main() { target := index priorityForm.Clear(true) - priorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:", 0, 2, false, true) + priorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:\nStatus:\nBlockNumber:", 0, 2, false, true) priorityForm.AddDropDown("Change priority to ->", priorities, index, func(priority string, selection int) { target = selection }) - priorityForm.AddButton("Save", func() { + priorityForm.AddButton("Update", func() { if target != index { seqManager.updatePriorityList(ctx, index, target) } + priorityForm.Clear(true) + seqManager.populateLists(ctx) + pages.SwitchToPage("Menu") + app.SetFocus(prioritySeqList) + }) + priorityForm.AddButton("Cancel", func() { + priorityForm.Clear(true) + pages.SwitchToPage("Menu") + app.SetFocus(prioritySeqList) + }) + priorityForm.AddButton("Remove", func() { + url := seqManager.priorityList[0] + delete(seqManager.prioritiesMap, url) + seqManager.updatePriorityList(ctx, index, 0) + seqManager.priorityList = seqManager.priorityList[1:] + + priorityForm.Clear(true) seqManager.populateLists(ctx) pages.SwitchToPage("Menu") + app.SetFocus(prioritySeqList) }) + priorityForm.SetFocus(1) + app.SetFocus(priorityForm) }) nonPrioritySeqList.SetSelectedFunc(func(index int, name string, second_name string, shortcut rune) { @@ -98,14 +118,30 @@ func main() { nonPriorityForm.AddDropDown("Set priority to ->", priorities, index, func(priority string, selection int) { target = selection }) - nonPriorityForm.AddButton("Save", func() { - seqManager.priorityList = append(seqManager.priorityList, seqManager.nonPriorityList[index]) + nonPriorityForm.AddButton("Update", func() { + key := seqManager.nonPriorityList[index] + seqManager.priorityList = append(seqManager.priorityList, key) + seqManager.prioritiesMap[key]++ + index = len(seqManager.priorityList) - 1 seqManager.updatePriorityList(ctx, index, target) + nonPriorityForm.Clear(true) seqManager.populateLists(ctx) pages.SwitchToPage("Menu") + if len(seqManager.nonPriorityList) > 0 { + app.SetFocus(nonPrioritySeqList) + } else { + app.SetFocus(prioritySeqList) + } + }) + nonPriorityForm.AddButton("Cancel", func() { + nonPriorityForm.Clear(true) + pages.SwitchToPage("Menu") + app.SetFocus(nonPrioritySeqList) }) + nonPriorityForm.SetFocus(1) + app.SetFocus(nonPriorityForm) }) // UI design @@ -118,18 +154,18 @@ func main() { SetText("-----Not in priority list but online-----") instructions := tview.NewTextView(). SetTextColor(tcell.ColorYellow). - SetText("(r) to refresh \n(a) to add sequencer\n(q) to quit") + SetText("(r) to refresh\n(s) to save all changes\n(c) to switch between lists\n(a) to add sequencer\n(q) to quit\n(tab) to navigate") flex.SetDirection(tview.FlexRow). AddItem(priorityHeading, 0, 1, false). AddItem(tview.NewFlex(). AddItem(prioritySeqList, 0, 2, true). - AddItem(priorityForm, 0, 3, false), 0, 12, false). + AddItem(priorityForm, 0, 3, true), 0, 12, true). AddItem(nonPriorityHeading, 0, 1, false). AddItem(tview.NewFlex(). AddItem(nonPrioritySeqList, 0, 2, true). - AddItem(nonPriorityForm, 0, 3, false), 0, 12, false). - AddItem(instructions, 0, 2, false).SetBorder(true) + AddItem(nonPriorityForm, 0, 3, true), 0, 12, true). + AddItem(instructions, 0, 3, false).SetBorder(true) flex.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey { if event.Rune() == 114 { @@ -138,10 +174,24 @@ func main() { nonPriorityForm.Clear(true) seqManager.populateLists(ctx) pages.SwitchToPage("Menu") + app.SetFocus(prioritySeqList) + } else if event.Rune() == 115 { + seqManager.pushUpdates(ctx) + priorityForm.Clear(true) + nonPriorityForm.Clear(true) + seqManager.populateLists(ctx) + pages.SwitchToPage("Menu") + app.SetFocus(prioritySeqList) } else if event.Rune() == 97 { addSeqForm.Clear(true) seqManager.addSeqPriorityForm(ctx) pages.SwitchToPage("Add Sequencer") + } else if event.Rune() == 99 { + if prioritySeqList.HasFocus() { + app.SetFocus(nonPrioritySeqList) + } else { + app.SetFocus(prioritySeqList) + } } else if event.Rune() == 113 { app.Stop() } @@ -164,11 +214,14 @@ func (sm *manager) updatePriorityList(ctx context.Context, index int, target int for i := index + 1; i <= target; i++ { sm.priorityList[i], sm.priorityList[i-1] = sm.priorityList[i-1], sm.priorityList[i] } - err := sm.redisCoordinator.UpdatePriorities(ctx, sm.priorityList) - if err != nil { - log.Warn("Failed to update priority, reverting change", "sequencer", sm.priorityList[target], "err", err) + + urlList := []string{} + for url := range sm.livelinessMap { + if _, ok := sm.prioritiesMap[url]; !ok { + urlList = append(urlList, url) + } } - sm.refreshAllLists(ctx) + sm.nonPriorityList = urlList } // populateLists populates seq's in priority list and seq's that are online but not in priority @@ -187,7 +240,7 @@ func (sm *manager) populateLists(ctx context.Context) { if _, ok := sm.livelinessMap[seqURL]; ok { status = fmt.Sprintf("%v ", emoji.GreenCircle) } - prioritySeqList.AddItem(status+seqURL+sec, "", rune(48+index), nil).SetSecondaryTextColor(tcell.ColorPurple) + prioritySeqList.AddItem(status+seqURL+sec, "", int32(48+index), nil).SetSecondaryTextColor(tcell.ColorPurple) } nonPrioritySeqList.Clear() @@ -212,11 +265,6 @@ func (sm *manager) addSeqPriorityForm(ctx context.Context) *tview.Form { // check if url is valid, i.e it doesnt already exist in the priority list if _, ok := sm.prioritiesMap[URL]; !ok && URL != "" { sm.priorityList = append(sm.priorityList, URL) - err := sm.redisCoordinator.UpdatePriorities(ctx, sm.priorityList) - if err != nil { - log.Warn("Failed to add sequencer to the priority list", URL) - } - sm.refreshAllLists(ctx) } sm.populateLists(ctx) pages.SwitchToPage("Menu") @@ -224,6 +272,15 @@ func (sm *manager) addSeqPriorityForm(ctx context.Context) *tview.Form { return addSeqForm } +// pushUpdates pushes the local changes to the redis server +func (sm *manager) pushUpdates(ctx context.Context) { + err := sm.redisCoordinator.UpdatePriorities(ctx, sm.priorityList) + if err != nil { + log.Warn("Failed to push local changes to the priority list") + } + sm.refreshAllLists(ctx) +} + // refreshAllLists gets the current status of all the lists displayed in the UI func (sm *manager) refreshAllLists(ctx context.Context) { sequencerURLList, mapping, err := sm.redisCoordinator.GetPriorities(ctx) From f8d72581bd06fd1221c75a70f92c4be8bb70dc03 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 21 Aug 2023 12:15:57 -0500 Subject: [PATCH 03/23] typo fix --- cmd/seq-coordinator-manager/seq-coordinator-manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index f39a810e9b..b8a4a47ac7 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -41,7 +41,7 @@ func main() { args := os.Args[1:] if len(args) != 1 { - fmt.Fprintf(os.Stderr, "Usage: redis-seq-manager [redis-url]\n") + fmt.Fprintf(os.Stderr, "Usage: seq-coordinator-manager [redis-url]\n") os.Exit(1) } redisURL := args[0] @@ -70,7 +70,7 @@ func main() { target := index priorityForm.Clear(true) - priorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:\nStatus:\nBlockNumber:", 0, 2, false, true) + priorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:", 0, 2, false, true) priorityForm.AddDropDown("Change priority to ->", priorities, index, func(priority string, selection int) { target = selection }) From 309bbb42d3d5733637eb993c67187b090787d6e2 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 21 Aug 2023 13:16:35 -0500 Subject: [PATCH 04/23] remove additional details and update list numbering to accomodate longer lists --- .../seq-coordinator-manager.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index b8a4a47ac7..eb28f6023c 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -70,7 +70,6 @@ func main() { target := index priorityForm.Clear(true) - priorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:", 0, 2, false, true) priorityForm.AddDropDown("Change priority to ->", priorities, index, func(priority string, selection int) { target = selection }) @@ -99,7 +98,7 @@ func main() { pages.SwitchToPage("Menu") app.SetFocus(prioritySeqList) }) - priorityForm.SetFocus(1) + priorityForm.SetFocus(0) app.SetFocus(priorityForm) }) @@ -114,7 +113,6 @@ func main() { target := index nonPriorityForm.Clear(true) - nonPriorityForm.AddTextView("Additional details:", "Status:\nBlockNumber:", 0, 2, false, true) nonPriorityForm.AddDropDown("Set priority to ->", priorities, index, func(priority string, selection int) { target = selection }) @@ -140,7 +138,7 @@ func main() { pages.SwitchToPage("Menu") app.SetFocus(nonPrioritySeqList) }) - nonPriorityForm.SetFocus(1) + nonPriorityForm.SetFocus(0) app.SetFocus(nonPriorityForm) }) @@ -236,17 +234,17 @@ func (sm *manager) populateLists(ctx context.Context) { if seqURL == chosen { sec = fmt.Sprintf(" %vchosen", emoji.LeftArrow) } - status := fmt.Sprintf("%v ", emoji.RedCircle) + status := fmt.Sprintf("(%d) %v ", index, emoji.RedCircle) if _, ok := sm.livelinessMap[seqURL]; ok { - status = fmt.Sprintf("%v ", emoji.GreenCircle) + status = fmt.Sprintf("(%d) %v ", index, emoji.GreenCircle) } - prioritySeqList.AddItem(status+seqURL+sec, "", int32(48+index), nil).SetSecondaryTextColor(tcell.ColorPurple) + prioritySeqList.AddItem(status+seqURL+sec, "", rune(0), nil).SetSecondaryTextColor(tcell.ColorPurple) } nonPrioritySeqList.Clear() - status := fmt.Sprintf("%v ", emoji.GreenCircle) + status := fmt.Sprintf("(-) %v ", emoji.GreenCircle) for _, seqURL := range sm.nonPriorityList { - nonPrioritySeqList.AddItem(status+seqURL, "", rune(45), nil) + nonPrioritySeqList.AddItem(status+seqURL, "", rune(0), nil) } } From 47e5e9f24821d1badcddf68cc60cfefedbb572d9 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Thu, 24 Aug 2023 10:33:30 -0500 Subject: [PATCH 05/23] fix minor bug --- cmd/seq-coordinator-manager/seq-coordinator-manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index eb28f6023c..96da53cc94 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -88,7 +88,7 @@ func main() { app.SetFocus(prioritySeqList) }) priorityForm.AddButton("Remove", func() { - url := seqManager.priorityList[0] + url := seqManager.priorityList[index] delete(seqManager.prioritiesMap, url) seqManager.updatePriorityList(ctx, index, 0) seqManager.priorityList = seqManager.priorityList[1:] From 90dd010b9427a9fb7f1f8b1919a14c1ae0ab1f94 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Thu, 24 Aug 2023 10:52:45 -0500 Subject: [PATCH 06/23] update recent merge conflict resolution --- go.sum | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go.sum b/go.sum index f7fdad4717..4362d4b01d 100644 --- a/go.sum +++ b/go.sum @@ -1973,6 +1973,9 @@ golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= From c5f6ba3ecb01ada623c5fe90e306ead00cb73d93 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Thu, 24 Aug 2023 13:48:16 -0500 Subject: [PATCH 07/23] update RedisCoordinator to inherit redisutil's implementation --- .../rediscoordinator/redis_coordinator.go | 25 +------------------ .../seq-coordinator-manager.go | 11 +++++--- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go index db3724240e..3dcb6f7203 100644 --- a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go +++ b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go @@ -10,18 +10,7 @@ import ( ) type RedisCoordinator struct { - Client redis.UniversalClient -} - -func NewRedisCoordinator(redisURL string) (*RedisCoordinator, error) { - redisClient, err := redisutil.RedisClientFromURL(redisURL) - if err != nil { - return nil, err - } - - return &RedisCoordinator{ - Client: redisClient, - }, nil + *redisutil.RedisCoordinator } func (rc *RedisCoordinator) GetPriorities(ctx context.Context) ([]string, map[string]int, error) { @@ -63,15 +52,3 @@ func (rc *RedisCoordinator) UpdatePriorities(ctx context.Context, priorities []s } return err } - -// CurrentChosenSequencer retrieves the current chosen sequencer holding the lock -func (c *RedisCoordinator) CurrentChosenSequencer(ctx context.Context) (string, error) { - current, err := c.Client.Get(ctx, redisutil.CHOSENSEQ_KEY).Result() - if errors.Is(err, redis.Nil) { - return "", nil - } - if err != nil { - return "", err - } - return current, nil -} diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index 96da53cc94..9bbafe18df 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -10,6 +10,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/gdamore/tcell/v2" "github.com/offchainlabs/nitro/cmd/seq-coordinator-manager/rediscoordinator" + "github.com/offchainlabs/nitro/util/redisutil" "github.com/rivo/tview" ) @@ -45,15 +46,17 @@ func main() { os.Exit(1) } redisURL := args[0] - redisCoordinator, err := rediscoordinator.NewRedisCoordinator(redisURL) + redisutilCoordinator, err := redisutil.NewRedisCoordinator(redisURL) if err != nil { panic(err) } seqManager := &manager{ - redisCoordinator: redisCoordinator, - prioritiesMap: make(map[string]int), - livelinessMap: make(map[string]int), + redisCoordinator: &rediscoordinator.RedisCoordinator{ + RedisCoordinator: redisutilCoordinator, + }, + prioritiesMap: make(map[string]int), + livelinessMap: make(map[string]int), } seqManager.refreshAllLists(ctx) From 6d7667715a5bf5311bf9b6289a11148724c075e8 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 25 Aug 2023 11:45:53 -0500 Subject: [PATCH 08/23] add comments --- .../rediscoordinator/redis_coordinator.go | 4 ++++ cmd/seq-coordinator-manager/seq-coordinator-manager.go | 1 + 2 files changed, 5 insertions(+) diff --git a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go index 3dcb6f7203..a393719a1d 100644 --- a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go +++ b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go @@ -9,10 +9,12 @@ import ( "github.com/offchainlabs/nitro/util/redisutil" ) +// RedisCoordinator builds upon RedisCoordinator of redisutil with additional functionality type RedisCoordinator struct { *redisutil.RedisCoordinator } +// GetPriorities returns the priority list of sequencers func (rc *RedisCoordinator) GetPriorities(ctx context.Context) ([]string, map[string]int, error) { prioritiesMap := make(map[string]int) prioritiesString, err := rc.Client.Get(ctx, redisutil.PRIORITIES_KEY).Result() @@ -29,6 +31,7 @@ func (rc *RedisCoordinator) GetPriorities(ctx context.Context) ([]string, map[st return priorities, prioritiesMap, nil } +// GetLivelinessMap returns a map whose keys are sequencers that have their liveliness set to OK func (rc *RedisCoordinator) GetLivelinessMap(ctx context.Context) (map[string]int, error) { livelinessMap := make(map[string]int) livelinessList, _, err := rc.Client.Scan(ctx, 0, redisutil.WANTS_LOCKOUT_KEY_PREFIX+"*", 0).Result() @@ -42,6 +45,7 @@ func (rc *RedisCoordinator) GetLivelinessMap(ctx context.Context) (map[string]in return livelinessMap, nil } +// UpdatePriorities updates the priority list of sequencers func (rc *RedisCoordinator) UpdatePriorities(ctx context.Context, priorities []string) error { prioritiesString := strings.Join(priorities, ",") err := rc.Client.Set(ctx, redisutil.PRIORITIES_KEY, prioritiesString, 0).Err() diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index 9bbafe18df..0a279cff64 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -265,6 +265,7 @@ func (sm *manager) addSeqPriorityForm(ctx context.Context) *tview.Form { addSeqForm.AddButton("Add", func() { // check if url is valid, i.e it doesnt already exist in the priority list if _, ok := sm.prioritiesMap[URL]; !ok && URL != "" { + sm.prioritiesMap[URL]++ sm.priorityList = append(sm.priorityList, URL) } sm.populateLists(ctx) From 2b01cf8fe37f115f548ae481e2deb34d6cbecef1 Mon Sep 17 00:00:00 2001 From: Nodar Date: Mon, 28 Aug 2023 17:34:31 +0200 Subject: [PATCH 09/23] Implement linter detection of koanf fields that aren't used outside flag definitions --- Makefile | 2 +- linter/koanf/handlers.go | 241 +++++++++++++++++++++++++++++++ linter/koanf/koanf.go | 201 ++++++-------------------- linter/koanf/koanf_test.go | 62 ++++++-- linter/testdata/src/a/a.go | 38 ----- linter/testdata/src/koanf/a/a.go | 58 ++++++++ linter/testdata/src/koanf/b/b.go | 52 +++++++ 7 files changed, 447 insertions(+), 207 deletions(-) create mode 100644 linter/koanf/handlers.go delete mode 100644 linter/testdata/src/a/a.go create mode 100644 linter/testdata/src/koanf/a/a.go create mode 100644 linter/testdata/src/koanf/b/b.go diff --git a/Makefile b/Makefile index 0d93958c2d..33487d0609 100644 --- a/Makefile +++ b/Makefile @@ -304,7 +304,7 @@ contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(arbitrator_pro # strategic rules to minimize dependency building .make/lint: $(DEP_PREDICATE) build-node-deps $(ORDER_ONLY_PREDICATE) .make - go run linter/koanf/koanf.go ./... + go run linter/koanf/koanf.go linter/koanf/handlers.go ./... go run linter/pointercheck/pointer.go ./... golangci-lint run --fix yarn --cwd contracts solhint diff --git a/linter/koanf/handlers.go b/linter/koanf/handlers.go new file mode 100644 index 0000000000..452291e605 --- /dev/null +++ b/linter/koanf/handlers.go @@ -0,0 +1,241 @@ +package main + +import ( + "fmt" + "go/ast" + "go/token" + "strings" + "unicode" + + "github.com/fatih/structtag" + "golang.org/x/tools/go/analysis" +) + +// handleComposite tracks use of fields in composite literals. +// E.g. `Config{A: 1, B: 2, C: 3}` will increase counters of fields A,B and C. +func handleComposite(pass *analysis.Pass, cl *ast.CompositeLit, cnt map[string]int) { + id, ok := cl.Type.(*ast.Ident) + if !ok { + return + } + for _, e := range cl.Elts { + if kv, ok := e.(*ast.KeyValueExpr); ok { + if ki, ok := kv.Key.(*ast.Ident); ok { + fi := pass.TypesInfo.Types[id].Type.String() + "." + ki.Name + cnt[normalizeID(pass, fi)]++ + } + } + } +} + +// handleSelector handles selector expression recursively, that is an expression: +// a.B.C.D will update counter for fields: a.B.C.D, a.B.C and a.B. +func handleSelector(pass *analysis.Pass, se *ast.SelectorExpr, inc int, cnt map[string]int) string { + if e, ok := se.X.(*ast.SelectorExpr); ok { + // Full field identifier, including package name. + fi := pass.TypesInfo.Types[e].Type.String() + "." + se.Sel.Name + cnt[normalizeID(pass, fi)] += inc + prefix := handleSelector(pass, e, inc, cnt) + fi = prefix + "." + se.Sel.Name + cnt[normalizeID(pass, fi)] += inc + return fi + } + // Handle selectors on function calls, e.g. `config().Enabled`. + if _, ok := se.X.(*ast.CallExpr); ok { + fi := pass.TypesInfo.Types[se.X].Type.String() + "." + se.Sel.Name + cnt[normalizeID(pass, fi)] += inc + return fi + } + if ident, ok := se.X.(*ast.Ident); ok { + if pass.TypesInfo.Types[ident].Type != nil { + fi := pass.TypesInfo.Types[ident].Type.String() + "." + se.Sel.Name + cnt[normalizeID(pass, fi)] += inc + return fi + } + } + return "" +} + +// koanfFields returns a map of fields that have koanf tag. +func koanfFields(pass *analysis.Pass) map[string]token.Pos { + res := make(map[string]token.Pos) + for _, f := range pass.Files { + pkgName := f.Name.Name + ast.Inspect(f, func(node ast.Node) bool { + if ts, ok := node.(*ast.TypeSpec); ok { + st, ok := ts.Type.(*ast.StructType) + if !ok { + return true + } + for _, f := range st.Fields.List { + if tag := tagFromField(f); tag != "" { + t := strings.Join([]string{pkgName, ts.Name.Name, f.Names[0].Name}, ".") + res[t] = f.Pos() + } + } + } + return true + }) + } + return res +} + +func containsFlagSet(params []*ast.Field) bool { + for _, p := range params { + se, ok := p.Type.(*ast.StarExpr) + if !ok { + continue + } + sle, ok := se.X.(*ast.SelectorExpr) + if !ok { + continue + } + if sle.Sel.Name == "FlagSet" { + return true + } + } + return false +} + +// checkFlagDefs checks flag definitions in the function. +// Result contains list of errors where flag name doesn't match field name. +func checkFlagDefs(pass *analysis.Pass, f *ast.FuncDecl, cnt map[string]int) Result { + // Ignore functions that does not get flagset as parameter. + if !containsFlagSet(f.Type.Params.List) { + return Result{} + } + var res Result + for _, s := range f.Body.List { + es, ok := s.(*ast.ExprStmt) + if !ok { + continue + } + callE, ok := es.X.(*ast.CallExpr) + if !ok { + continue + } + if len(callE.Args) != 3 { + continue + } + sl, ok := extractStrLit(callE.Args[0]) + if !ok { + continue + } + s, ok := selectorName(callE.Args[1]) + if !ok { + continue + } + handleSelector(pass, callE.Args[1].(*ast.SelectorExpr), -1, cnt) + if normSL := normalizeTag(sl); !strings.EqualFold(normSL, s) { + res.Errors = append(res.Errors, koanfError{ + Pos: f.Pos(), + Message: fmt.Sprintf("koanf tag name: %q doesn't match the field: %q", sl, s), + err: errIncorrectFlag, + }) + } + + } + return res +} + +func selectorName(e ast.Expr) (string, bool) { + n, ok := e.(ast.Node) + if !ok { + return "", false + } + se, ok := n.(*ast.SelectorExpr) + if !ok { + return "", false + } + return se.Sel.Name, true +} + +// Extracts literal from expression that is either: +// - string literal or +// - sum of variable and string literal. +// E.g. +// strLitFromSum(`"max-size"`) = "max-size" +// - strLitFromSum(`prefix + ".enable"“) = ".enable". +func extractStrLit(e ast.Expr) (string, bool) { + if s, ok := strLit(e); ok { + return s, true + } + if be, ok := e.(*ast.BinaryExpr); ok { + if be.Op == token.ADD { + if s, ok := strLit(be.Y); ok { + // Drop the prefix dot. + return s[1:], true + } + } + } + return "", false +} + +func strLit(e ast.Expr) (string, bool) { + if s, ok := e.(*ast.BasicLit); ok { + if s.Kind == token.STRING { + return strings.Trim(s.Value, "\""), true + } + } + return "", false +} + +// tagFromField extracts koanf tag from struct field. +func tagFromField(f *ast.Field) string { + if f.Tag == nil { + return "" + } + tags, err := structtag.Parse(strings.Trim((f.Tag.Value), "`")) + if err != nil { + return "" + } + tag, err := tags.Get("koanf") + if err != nil { + return "" + } + return normalizeTag(tag.Name) +} + +// checkStruct returns violations where koanf tag name doesn't match field names. +func checkStruct(pass *analysis.Pass, s *ast.StructType) Result { + var res Result + for _, f := range s.Fields.List { + tag := tagFromField(f) + if tag == "" { + continue + } + fieldName := f.Names[0].Name + if !strings.EqualFold(tag, fieldName) { + res.Errors = append(res.Errors, koanfError{ + Pos: f.Pos(), + Message: fmt.Sprintf("field name: %q doesn't match tag name: %q\n", fieldName, tag), + err: errMismatch, + }) + } + } + return res +} + +func normalizeTag(s string) string { + ans := s[:1] + for i := 1; i < len(s); i++ { + c := rune(s[i]) + if !isAlphanumeric(c) { + continue + } + if !isAlphanumeric(rune(s[i-1])) && unicode.IsLower(c) { + c = unicode.ToUpper(c) + } + ans += string(c) + } + return ans +} + +func isAlphanumeric(c rune) bool { + return unicode.IsLetter(c) || unicode.IsDigit(c) +} + +func normalizeID(pass *analysis.Pass, id string) string { + id = strings.TrimPrefix(id, "*") + return pass.Pkg.Name() + strings.TrimPrefix(id, pass.Pkg.Path()) +} diff --git a/linter/koanf/koanf.go b/linter/koanf/koanf.go index 8dbb392cb4..d6780760e7 100644 --- a/linter/koanf/koanf.go +++ b/linter/koanf/koanf.go @@ -1,18 +1,23 @@ package main import ( + "errors" "fmt" "go/ast" "go/token" "reflect" - "strings" - "unicode" - "github.com/fatih/structtag" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/singlechecker" ) +var ( + errUnused = errors.New("unused") + errMismatch = errors.New("mismmatched field name and tag in a struct") + // e.g. f.Int("max-sz", DefaultBatchPosterConfig.MaxSize, "maximum batch size") + errIncorrectFlag = errors.New("mismatching flag initialization") +) + func New(conf any) ([]*analysis.Analyzer, error) { return []*analysis.Analyzer{Analyzer}, nil } @@ -33,8 +38,9 @@ var analyzerForTests = &analysis.Analyzer{ // koanfError indicates the position of an error in configuration. type koanfError struct { - Pos token.Position + Pos token.Pos Message string + err error } // Result is returned from the checkStruct function, and holds all the @@ -44,7 +50,14 @@ type Result struct { } func run(dryRun bool, pass *analysis.Pass) (interface{}, error) { - var ret Result + var ( + ret Result + cnt = make(map[string]int) + // koanfFields map contains all the struct koanfFields that have koanf tag. + // It identifies field as "{pkgName}.{structName}.{field_Name}". + // e.g. "a.BatchPosterConfig.Enable", "a.BatchPosterConfig.MaxSize" + koanfFields = koanfFields(pass) + ) for _, f := range pass.Files { ast.Inspect(f, func(node ast.Node) bool { var res Result @@ -52,167 +65,41 @@ func run(dryRun bool, pass *analysis.Pass) (interface{}, error) { case *ast.StructType: res = checkStruct(pass, v) case *ast.FuncDecl: - res = checkFlagDefs(pass, v) - default: - } - for _, err := range res.Errors { - ret.Errors = append(ret.Errors, err) - if !dryRun { - pass.Report(analysis.Diagnostic{ - Pos: pass.Fset.File(f.Pos()).Pos(err.Pos.Offset), - Message: err.Message, - Category: "koanf", - }) + res = checkFlagDefs(pass, v, cnt) + case *ast.SelectorExpr: + handleSelector(pass, v, 1, cnt) + case *ast.IfStmt: + if se, ok := v.Cond.(*ast.SelectorExpr); ok { + handleSelector(pass, se, 1, cnt) } + case *ast.CompositeLit: + handleComposite(pass, v, cnt) + default: } + ret.Errors = append(ret.Errors, res.Errors...) return true - }, - ) - } - return ret, nil -} - -func containsFlagSet(params []*ast.Field) bool { - for _, p := range params { - se, ok := p.Type.(*ast.StarExpr) - if !ok { - continue - } - sle, ok := se.X.(*ast.SelectorExpr) - if !ok { - continue - } - if sle.Sel.Name == "FlagSet" { - return true - } - } - return false -} - -// checkFlagDefs checks flag definitions in the function. -// Result contains list of errors where flag name doesn't match field name. -func checkFlagDefs(pass *analysis.Pass, f *ast.FuncDecl) Result { - // Ignore functions that does not get flagset as parameter. - if !containsFlagSet(f.Type.Params.List) { - return Result{} - } - var res Result - for _, s := range f.Body.List { - es, ok := s.(*ast.ExprStmt) - if !ok { - continue - } - callE, ok := es.X.(*ast.CallExpr) - if !ok { - continue - } - if len(callE.Args) != 3 { - continue - } - sl, ok := extractStrLit(callE.Args[0]) - if !ok { - continue - } - s, ok := selector(callE.Args[1]) - if !ok { - continue - } - if normSL := normalize(sl); !strings.EqualFold(normSL, s) { - res.Errors = append(res.Errors, koanfError{ - Pos: pass.Fset.Position(f.Pos()), - Message: fmt.Sprintf("koanf tag name: %q doesn't match the field: %q", sl, s), - }) - } - + }) } - return res -} - -func selector(e ast.Expr) (string, bool) { - n, ok := e.(ast.Node) - if !ok { - return "", false - } - se, ok := n.(*ast.SelectorExpr) - if !ok { - return "", false - } - return se.Sel.Name, true -} - -// Extracts literal from expression that is either: -// - string literal or -// - sum of variable and string literal. -// E.g. -// strLitFromSum(`"max-size"`) = "max-size" -// - strLitFromSum(`prefix + ".enable"“) = ".enable". -func extractStrLit(e ast.Expr) (string, bool) { - if s, ok := strLit(e); ok { - return s, true - } - if be, ok := e.(*ast.BinaryExpr); ok { - if be.Op == token.ADD { - if s, ok := strLit(be.Y); ok { - // Drop the prefix dot. - return s[1:], true - } - } - } - return "", false -} - -func strLit(e ast.Expr) (string, bool) { - if s, ok := e.(*ast.BasicLit); ok { - if s.Kind == token.STRING { - return strings.Trim(s.Value, "\""), true + for k := range koanfFields { + if cnt[k] == 0 { + ret.Errors = append(ret.Errors, + koanfError{ + Pos: koanfFields[k], + Message: fmt.Sprintf("field %v not used", k), + err: errUnused, + }) } } - return "", false -} - -func checkStruct(pass *analysis.Pass, s *ast.StructType) Result { - var res Result - for _, f := range s.Fields.List { - if f.Tag == nil { - continue - } - tags, err := structtag.Parse(strings.Trim((f.Tag.Value), "`")) - if err != nil { - continue - } - tag, err := tags.Get("koanf") - if err != nil { - continue - } - tagName := normalize(tag.Name) - fieldName := f.Names[0].Name - if !strings.EqualFold(tagName, fieldName) { - res.Errors = append(res.Errors, koanfError{ - Pos: pass.Fset.Position(f.Pos()), - Message: fmt.Sprintf("field name: %q doesn't match tag name: %q\n", fieldName, tagName), + for _, err := range ret.Errors { + if !dryRun { + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + Message: err.Message, + Category: "koanf", }) } } - return res -} - -func normalize(s string) string { - ans := s[:1] - for i := 1; i < len(s); i++ { - c := rune(s[i]) - if !isAlphanumeric(c) { - continue - } - if !isAlphanumeric(rune(s[i-1])) && unicode.IsLower(c) { - c = unicode.ToUpper(c) - } - ans += string(c) - } - return ans -} - -func isAlphanumeric(c rune) bool { - return unicode.IsLetter(c) || unicode.IsDigit(c) + return ret, nil } func main() { diff --git a/linter/koanf/koanf_test.go b/linter/koanf/koanf_test.go index e3ad5e6043..064ae533c4 100644 --- a/linter/koanf/koanf_test.go +++ b/linter/koanf/koanf_test.go @@ -1,31 +1,71 @@ package main import ( + "errors" "os" "path/filepath" "testing" + "github.com/google/go-cmp/cmp" "golang.org/x/tools/go/analysis/analysistest" ) -func TestAll(t *testing.T) { +var ( + incorrectFlag = "incorrect_flag" + mismatch = "mismatch" + unused = "unused" +) + +func testData(t *testing.T) string { + t.Helper() wd, err := os.Getwd() if err != nil { t.Fatalf("Failed to get wd: %s", err) } - testdata := filepath.Join(filepath.Dir(wd), "testdata") - res := analysistest.Run(t, testdata, analyzerForTests, "a") - if cnt := countErrors(res); cnt != 3 { - t.Errorf("analysistest.Run() got %v errors, expected 3", cnt) + return filepath.Join(filepath.Dir(wd), "testdata") +} + +// Tests koanf/a package that contains two types of errors where: +// - koanf tag doesn't match field name. +// - flag definition doesn't match field name. +// Errors are marked as comments in the package source file. +func TestMismatch(t *testing.T) { + testdata := testData(t) + got := errCounts(analysistest.Run(t, testdata, analyzerForTests, "koanf/a")) + want := map[string]int{ + incorrectFlag: 2, + mismatch: 1, + } + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("analysistest.Run() unexpected diff:\n%s\n", diff) + } +} + +func TestUnused(t *testing.T) { + testdata := testData(t) + got := errCounts(analysistest.Run(t, testdata, analyzerForTests, "koanf/b")) + if diff := cmp.Diff(got, map[string]int{"unused": 2}); diff != "" { + t.Errorf("analysistest.Run() unexpected diff:\n%s\n", diff) } } -func countErrors(errs []*analysistest.Result) int { - cnt := 0 - for _, e := range errs { - if r, ok := e.Result.(Result); ok { - cnt += len(r.Errors) +func errCounts(res []*analysistest.Result) map[string]int { + m := make(map[string]int) + for _, r := range res { + if rs, ok := r.Result.(Result); ok { + for _, e := range rs.Errors { + var s string + switch { + case errors.Is(e.err, errIncorrectFlag): + s = incorrectFlag + case errors.Is(e.err, errMismatch): + s = mismatch + case errors.Is(e.err, errUnused): + s = unused + } + m[s] = m[s] + 1 + } } } - return cnt + return m } diff --git a/linter/testdata/src/a/a.go b/linter/testdata/src/a/a.go deleted file mode 100644 index 86b7739108..0000000000 --- a/linter/testdata/src/a/a.go +++ /dev/null @@ -1,38 +0,0 @@ -package a - -import ( - "flag" -) - -type Config struct { - // Field name doesn't match koanf tag. - L2 int `koanf:"chain"` - LogLevel int `koanf:"log-level"` - LogType int `koanf:"log-type"` - Metrics int `koanf:"metrics"` - PProf int `koanf:"pprof"` - Node int `koanf:"node"` - Queue int `koanf:"queue"` -} - -type BatchPosterConfig struct { - Enable bool `koanf:"enable"` - MaxSize int `koanf:"max-size" reload:"hot"` -} - -// Flag names don't match field names from default config. -// Contains 2 errors. -func BatchPosterConfigAddOptions(prefix string, f *flag.FlagSet) { - f.Bool(prefix+".enabled", DefaultBatchPosterConfig.Enable, "enable posting batches to l1") - f.Int("max-sz", DefaultBatchPosterConfig.MaxSize, "maximum batch size") -} - -func ConfigAddOptions(prefix string, f *flag.FlagSet) { - f.Bool(prefix+".enable", DefaultBatchPosterConfig.Enable, "enable posting batches to l1") - f.Int("max-size", DefaultBatchPosterConfig.MaxSize, "maximum batch size") -} - -var DefaultBatchPosterConfig = BatchPosterConfig{ - Enable: false, - MaxSize: 100000, -} diff --git a/linter/testdata/src/koanf/a/a.go b/linter/testdata/src/koanf/a/a.go new file mode 100644 index 0000000000..a0513fb09b --- /dev/null +++ b/linter/testdata/src/koanf/a/a.go @@ -0,0 +1,58 @@ +package a + +import ( + "flag" +) + +type Config struct { + L2 int `koanf:"chain"` // Err: mismatch. + LogLevel int `koanf:"log-level"` + LogType int `koanf:"log-type"` + Metrics int `koanf:"metrics"` + PProf int `koanf:"pprof"` + Node int `koanf:"node"` + Queue int `koanf:"queue"` +} + +// Cover using of all fields in a various way: + +// Instantiating a type. +var defaultConfig = Config{ + L2: 1, + LogLevel: 2, +} + +// Instantiating a type an taking reference. +var defaultConfigPtr = &Config{ + LogType: 3, + Metrics: 4, +} + +func init() { + defaultConfig.PProf = 5 + defaultConfig.Node, _ = 6, 0 + defaultConfigPtr.Queue = 7 +} + +type BatchPosterConfig struct { + Enable bool `koanf:"enable"` + MaxSize int `koanf:"max-size" reload:"hot"` +} + +var DefaultBatchPosterConfig BatchPosterConfig + +func BatchPosterConfigAddOptions(prefix string, f *flag.FlagSet) { + f.Bool(prefix+".enabled", DefaultBatchPosterConfig.Enable, "") // Err: incorrect flag. + f.Int("max-sz", DefaultBatchPosterConfig.MaxSize, "") // Err: incorrect flag. +} + +func ConfigAddOptions(prefix string, f *flag.FlagSet) { + f.Bool(prefix+".enable", DefaultBatchPosterConfig.Enable, "enable posting batches to l1") + f.Int("max-size", DefaultBatchPosterConfig.MaxSize, "maximum batch size") +} + +func init() { + // Fields must be used outside flag definitions at least once. + DefaultBatchPosterConfig.Enable = true + DefaultBatchPosterConfig.MaxSize = 3 +} diff --git a/linter/testdata/src/koanf/b/b.go b/linter/testdata/src/koanf/b/b.go new file mode 100644 index 0000000000..fe958f17b3 --- /dev/null +++ b/linter/testdata/src/koanf/b/b.go @@ -0,0 +1,52 @@ +package b + +import ( + "flag" + "fmt" +) + +type ParCfg struct { + child ChildCfg `koanf:"child"` + grandChild GrandChildCfg `koanf:grandchild` +} + +var defaultCfg = ParCfg{} + +type ChildCfg struct { + A bool `koanf:"A"` + B bool `koanf:"B"` + C bool `koanf:"C"` + D bool `koanf:"D"` // Error: not used outside flag definition. +} + +var defaultChildCfg = ChildCfg{} + +func childConfigAddOptions(prefix string, f *flag.FlagSet) { + f.Bool(prefix+".a", defaultChildCfg.A, "") + f.Bool("b", defaultChildCfg.B, "") + f.Bool("c", defaultChildCfg.C, "") + f.Bool("d", defaultChildCfg.D, "") +} + +type GrandChildCfg struct { + A int `koanf:"A"` // Error: unused. +} + +func (c *GrandChildCfg) Do() { +} + +func configPtr() *ChildCfg { + return nil +} +func config() ChildCfg { + return ChildCfg{} +} + +func init() { + fmt.Printf("%v %v", config().A, configPtr().B) + // This covers usage of both `ParCfg.Child` and `ChildCfg.C`. + _ = defaultCfg.child.C + // Covers usage of grandChild. + defaultCfg.grandChild.Do() + +} From 80dc09da065f87fb5bc740dd820d687cc0942a38 Mon Sep 17 00:00:00 2001 From: Nodar Date: Mon, 28 Aug 2023 17:54:25 +0200 Subject: [PATCH 10/23] Enable custom linters (koanf/pointercheck) in CI --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d2f5765d72..5b0b33848c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,6 +129,10 @@ jobs: version: latest skip-go-installation: true skip-pkg-cache: true + - name: Custom Lint + run: | + go run linter/koanf/koanf.go linter/koanf/handlers.go ./... + go run linter/pointercheck/pointer.go ./... - name: Set environment variables run: | From 10af61eb84e7d4e9eaa263794c53c0f14da09fc2 Mon Sep 17 00:00:00 2001 From: Nodar Date: Mon, 28 Aug 2023 17:55:35 +0200 Subject: [PATCH 11/23] Fix whitespace in ci.yml --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b0b33848c..916969f324 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,7 +132,7 @@ jobs: - name: Custom Lint run: | go run linter/koanf/koanf.go linter/koanf/handlers.go ./... - go run linter/pointercheck/pointer.go ./... + go run linter/pointercheck/pointer.go ./... - name: Set environment variables run: | From 64b6adb383227dfc47b4d10f5cfb34d76c1dc6a1 Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 29 Aug 2023 20:03:52 +0200 Subject: [PATCH 12/23] Implement structinit linter that checks for marked structs that all instantiations initialize all the fields --- Makefile | 1 + linter/structinit/structinit.go | 122 ++++++++++++++++++++++++++ linter/structinit/structinit_test.go | 36 ++++++++ linter/testdata/src/structinit/a/a.go | 28 ++++++ 4 files changed, 187 insertions(+) create mode 100644 linter/structinit/structinit.go create mode 100644 linter/structinit/structinit_test.go create mode 100644 linter/testdata/src/structinit/a/a.go diff --git a/Makefile b/Makefile index 33487d0609..9f1c2b37e7 100644 --- a/Makefile +++ b/Makefile @@ -306,6 +306,7 @@ contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(arbitrator_pro .make/lint: $(DEP_PREDICATE) build-node-deps $(ORDER_ONLY_PREDICATE) .make go run linter/koanf/koanf.go linter/koanf/handlers.go ./... go run linter/pointercheck/pointer.go ./... + go run linter/structinit/structinit.go ./... golangci-lint run --fix yarn --cwd contracts solhint @touch $@ diff --git a/linter/structinit/structinit.go b/linter/structinit/structinit.go new file mode 100644 index 0000000000..e4e65bc3fc --- /dev/null +++ b/linter/structinit/structinit.go @@ -0,0 +1,122 @@ +package main + +import ( + "fmt" + "go/ast" + "go/token" + "reflect" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/singlechecker" +) + +// Tip for linter that struct that has this comment should be included in the +// analysis. +// Note: comment should be directly line above the struct definition. +const linterTip = "// lint:require-exhaustive-initialization" + +func New(conf any) ([]*analysis.Analyzer, error) { + return []*analysis.Analyzer{Analyzer}, nil +} + +// Analyzer implements struct analyzer for structs that are annotated with +// `linterTip`, it checks that every instantiation initializes all the fields. +var Analyzer = &analysis.Analyzer{ + Name: "structinit", + Doc: "check for struct field initializations", + Run: func(p *analysis.Pass) (interface{}, error) { return run(false, p) }, + ResultType: reflect.TypeOf(Result{}), +} + +var analyzerForTests = &analysis.Analyzer{ + Name: "teststructinit", + Doc: "check for struct field initializations", + Run: func(p *analysis.Pass) (interface{}, error) { return run(true, p) }, + ResultType: reflect.TypeOf(Result{}), +} + +type structError struct { + Pos token.Pos + Message string +} + +type Result struct { + Errors []structError +} + +func run(dryRun bool, pass *analysis.Pass) (interface{}, error) { + var ( + ret Result + structs = markedStructs(pass) + ) + for _, f := range pass.Files { + ast.Inspect(f, func(node ast.Node) bool { + // For every composite literal check that number of elements in + // the literal match the number of struct fields. + if cl, ok := node.(*ast.CompositeLit); ok { + stName := pass.TypesInfo.Types[cl].Type.String() + if cnt, found := structs[stName]; found && cnt != len(cl.Elts) { + ret.Errors = append(ret.Errors, structError{ + Pos: cl.Pos(), + Message: fmt.Sprintf("struct: %q initialized with: %v of total: %v fields", stName, len(cl.Elts), cnt), + }) + + } + + } + return true + }) + } + for _, err := range ret.Errors { + if !dryRun { + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + Message: err.Message, + Category: "structinit", + }) + } + } + return ret, nil +} + +// markedStructs returns a map of structs that are annotated for linter to check +// that all fields are initialized when the struct is instantiated. +// It maps struct full name (including package path) to number of fields it contains. +func markedStructs(pass *analysis.Pass) map[string]int { + res := make(map[string]int) + for _, f := range pass.Files { + tips := make(map[position]bool) + ast.Inspect(f, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.Comment: + p := pass.Fset.Position(node.Pos()) + if strings.Contains(n.Text, linterTip) { + tips[position{p.Filename, p.Line + 1}] = true + } + case *ast.TypeSpec: + if st, ok := n.Type.(*ast.StructType); ok { + p := pass.Fset.Position(st.Struct) + if tips[position{p.Filename, p.Line}] { + fieldsCnt := 0 + for _, field := range st.Fields.List { + fieldsCnt += len(field.Names) + } + res[pass.Pkg.Path()+"."+n.Name.Name] = fieldsCnt + } + } + } + return true + }) + } + return res +} + +type position struct { + fileName string + line int +} + +func main() { + singlechecker.Main(Analyzer) +} diff --git a/linter/structinit/structinit_test.go b/linter/structinit/structinit_test.go new file mode 100644 index 0000000000..db3676e185 --- /dev/null +++ b/linter/structinit/structinit_test.go @@ -0,0 +1,36 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func testData(t *testing.T) string { + t.Helper() + wd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get wd: %s", err) + } + return filepath.Join(filepath.Dir(wd), "testdata") +} + +func TestLinter(t *testing.T) { + testdata := testData(t) + got := errCount(analysistest.Run(t, testdata, analyzerForTests, "structinit/a")) + if got != 2 { + t.Errorf("analysistest.Run() got %d errors, expected 2", got) + } +} + +func errCount(res []*analysistest.Result) int { + cnt := 0 + for _, r := range res { + if rs, ok := r.Result.(Result); ok { + cnt += len(rs.Errors) + } + } + return cnt +} diff --git a/linter/testdata/src/structinit/a/a.go b/linter/testdata/src/structinit/a/a.go new file mode 100644 index 0000000000..40be4dea21 --- /dev/null +++ b/linter/testdata/src/structinit/a/a.go @@ -0,0 +1,28 @@ +package a + +import "fmt" + +// lint:require-exhaustive-initialization +type interestingStruct struct { + x int + b *boringStruct +} + +type boringStruct struct { + x, y int +} + +func init() { + a := &interestingStruct{ + x: 1, // Error: only single field is initialized. + } + fmt.Println(a) + b := interestingStruct{ + b: nil, // Error: only single field is initialized. + } + fmt.Println(b) + c := &boringStruct{ + x: 1, // Not an error since it's not annotated for the linter. + } + fmt.Println(c) +} From 89d96b6ffaf1b2a2b97c27a08885b86a83e2e993 Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 29 Aug 2023 20:06:18 +0200 Subject: [PATCH 13/23] Add testcase to structinit linter --- linter/testdata/src/structinit/a/a.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/linter/testdata/src/structinit/a/a.go b/linter/testdata/src/structinit/a/a.go index 40be4dea21..45f6059726 100644 --- a/linter/testdata/src/structinit/a/a.go +++ b/linter/testdata/src/structinit/a/a.go @@ -13,16 +13,21 @@ type boringStruct struct { } func init() { - a := &interestingStruct{ - x: 1, // Error: only single field is initialized. + a := &interestingStruct{ // Error: only single field is initialized. + x: 1, } fmt.Println(a) - b := interestingStruct{ - b: nil, // Error: only single field is initialized. + b := interestingStruct{ // Error: only single field is initialized. + b: nil, } fmt.Println(b) - c := &boringStruct{ - x: 1, // Not an error since it's not annotated for the linter. + c := interestingStruct{ // Not an error, all fields are initialized. + x: 1, + b: nil, } fmt.Println(c) + d := &boringStruct{ // Not an error since it's not annotated for the linter. + x: 1, + } + fmt.Println(d) } From daac2b8f21e2c7837a65c12d8f8294403dc54b25 Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 30 Aug 2023 15:37:33 +0200 Subject: [PATCH 14/23] Add comment about increaseBy parameter --- linter/koanf/handlers.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/linter/koanf/handlers.go b/linter/koanf/handlers.go index 452291e605..00cd10c07e 100644 --- a/linter/koanf/handlers.go +++ b/linter/koanf/handlers.go @@ -30,26 +30,28 @@ func handleComposite(pass *analysis.Pass, cl *ast.CompositeLit, cnt map[string]i // handleSelector handles selector expression recursively, that is an expression: // a.B.C.D will update counter for fields: a.B.C.D, a.B.C and a.B. -func handleSelector(pass *analysis.Pass, se *ast.SelectorExpr, inc int, cnt map[string]int) string { +// It updates counters map in place, increasing corresponding identifiers by +// increaseBy amount. +func handleSelector(pass *analysis.Pass, se *ast.SelectorExpr, increaseBy int, cnt map[string]int) string { if e, ok := se.X.(*ast.SelectorExpr); ok { // Full field identifier, including package name. fi := pass.TypesInfo.Types[e].Type.String() + "." + se.Sel.Name - cnt[normalizeID(pass, fi)] += inc - prefix := handleSelector(pass, e, inc, cnt) + cnt[normalizeID(pass, fi)] += increaseBy + prefix := handleSelector(pass, e, increaseBy, cnt) fi = prefix + "." + se.Sel.Name - cnt[normalizeID(pass, fi)] += inc + cnt[normalizeID(pass, fi)] += increaseBy return fi } // Handle selectors on function calls, e.g. `config().Enabled`. if _, ok := se.X.(*ast.CallExpr); ok { fi := pass.TypesInfo.Types[se.X].Type.String() + "." + se.Sel.Name - cnt[normalizeID(pass, fi)] += inc + cnt[normalizeID(pass, fi)] += increaseBy return fi } if ident, ok := se.X.(*ast.Ident); ok { if pass.TypesInfo.Types[ident].Type != nil { fi := pass.TypesInfo.Types[ident].Type.String() + "." + se.Sel.Name - cnt[normalizeID(pass, fi)] += inc + cnt[normalizeID(pass, fi)] += increaseBy return fi } } From 4d2118660a75fb1dcf04491e07049a2041d3fd83 Mon Sep 17 00:00:00 2001 From: Nodar Date: Thu, 31 Aug 2023 15:38:54 +0200 Subject: [PATCH 15/23] drop tag normalization in koanf.go --- linter/koanf/handlers.go | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/linter/koanf/handlers.go b/linter/koanf/handlers.go index 2381a230ee..5826004014 100644 --- a/linter/koanf/handlers.go +++ b/linter/koanf/handlers.go @@ -5,7 +5,6 @@ import ( "go/ast" "go/token" "strings" - "unicode" "github.com/fatih/structtag" "golang.org/x/tools/go/analysis" @@ -195,7 +194,7 @@ func tagFromField(f *ast.Field) string { if err != nil { return "" } - return strings.ReplaceAll(tag.Name, "-", "") + return normalizeTag(tag.Name) } // checkStruct returns violations where koanf tag name doesn't match field names. @@ -219,22 +218,7 @@ func checkStruct(pass *analysis.Pass, s *ast.StructType) Result { } func normalizeTag(s string) string { - ans := s[:1] - for i := 1; i < len(s); i++ { - c := rune(s[i]) - if !isAlphanumeric(c) { - continue - } - if !isAlphanumeric(rune(s[i-1])) && unicode.IsLower(c) { - c = unicode.ToUpper(c) - } - ans += string(c) - } - return ans -} - -func isAlphanumeric(c rune) bool { - return unicode.IsLetter(c) || unicode.IsDigit(c) + return strings.ReplaceAll(s, "-", "") } func normalizeID(pass *analysis.Pass, id string) string { From 293ed4c504e3370acb0984cac36db02dd1e82275 Mon Sep 17 00:00:00 2001 From: Nodar Date: Fri, 1 Sep 2023 17:14:06 +0200 Subject: [PATCH 16/23] Specify folders of linters instead of separate go files in CI yml --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 916969f324..a0f5251f9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,8 +131,8 @@ jobs: skip-pkg-cache: true - name: Custom Lint run: | - go run linter/koanf/koanf.go linter/koanf/handlers.go ./... - go run linter/pointercheck/pointer.go ./... + go run ./linter/koanf ./... + go run ./linter/pointercheck ./... - name: Set environment variables run: | From 4be9bbb019fde7e6103304f266a85e0622f3b7b8 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 1 Sep 2023 13:24:48 -0500 Subject: [PATCH 17/23] remove precompilesgen dependency from headerreader --- arbnode/node.go | 7 +++++-- cmd/daserver/daserver.go | 5 ++++- cmd/nitro/nitro.go | 5 ++++- system_tests/das_test.go | 4 +++- util/headerreader/header_reader.go | 17 +++++++++-------- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index e6960a3f22..e3e9223b1d 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -40,6 +40,7 @@ import ( "github.com/offchainlabs/nitro/solgen/go/bridgegen" "github.com/offchainlabs/nitro/solgen/go/challengegen" "github.com/offchainlabs/nitro/solgen/go/ospgen" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/solgen/go/rollupgen" "github.com/offchainlabs/nitro/staker" "github.com/offchainlabs/nitro/util/contracts" @@ -235,7 +236,8 @@ func GenerateRollupConfig(prod bool, wasmModuleRoot common.Hash, rollupOwner com } func DeployOnL1(ctx context.Context, l1client arbutil.L1Interface, deployAuth *bind.TransactOpts, batchPoster common.Address, authorizeValidators uint64, readerConfig headerreader.ConfigFetcher, config rollupgen.Config) (*chaininfo.RollupAddresses, error) { - l1Reader, err := headerreader.New(ctx, l1client, readerConfig) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, readerConfig, arbSys) if err != nil { return nil, err } @@ -611,7 +613,8 @@ func createNodeImpl( var l1Reader *headerreader.HeaderReader if config.ParentChainReader.Enable { - l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys) if err != nil { return nil, err } diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go index 7cdfc39915..335aba6a1b 100644 --- a/cmd/daserver/daserver.go +++ b/cmd/daserver/daserver.go @@ -17,6 +17,7 @@ import ( flag "github.com/spf13/pflag" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/metrics/exp" @@ -24,6 +25,7 @@ import ( "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/cmd/util/confighelpers" "github.com/offchainlabs/nitro/das" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/headerreader" ) @@ -196,7 +198,8 @@ func startup() error { if err != nil { return err } - l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }) // TODO: config + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) + l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys) // TODO: config if err != nil { return err } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index 407ed0afe7..dd26fea46f 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -28,6 +28,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/arbitrum" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" _ "github.com/ethereum/go-ethereum/eth/tracers/js" _ "github.com/ethereum/go-ethereum/eth/tracers/native" @@ -48,6 +49,7 @@ import ( "github.com/offchainlabs/nitro/cmd/util" "github.com/offchainlabs/nitro/cmd/util/confighelpers" _ "github.com/offchainlabs/nitro/nodeInterface" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/staker" "github.com/offchainlabs/nitro/util/colors" "github.com/offchainlabs/nitro/util/headerreader" @@ -354,7 +356,8 @@ func mainImpl() int { flag.Usage() log.Crit("--node.validator.only-create-wallet-contract requires --node.validator.use-smart-contract-wallet") } - l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) + l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys) if err != nil { log.Crit("failed to get L1 headerreader", "error", err) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 7952120933..8889d2d53d 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -28,6 +28,7 @@ import ( "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/das" "github.com/offchainlabs/nitro/solgen/go/bridgegen" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/util/signature" ) @@ -233,7 +234,8 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { chainConfig := params.ArbitrumDevTestDASChainConfig() l1info, l1client, _, l1stack := createTestL1BlockChain(t, nil) defer requireClose(t, l1stack) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index e5807224c0..8487ccd54b 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -18,17 +18,20 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rpc" "github.com/offchainlabs/nitro/arbutil" - "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/stopwaiter" flag "github.com/spf13/pflag" ) +type ArbSysInterface interface { + ArbBlockNumber(*bind.CallOpts) (*big.Int, error) +} + type HeaderReader struct { stopwaiter.StopWaiter config ConfigFetcher client arbutil.L1Interface isParentChainArbitrum bool - arbSys *precompilesgen.ArbSys + arbSys ArbSysInterface chanMutex sync.RWMutex // All fields below require the chanMutex @@ -91,25 +94,23 @@ var TestConfig = Config{ UseFinalityData: false, } -func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher) (*HeaderReader, error) { +func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface) (*HeaderReader, error) { isParentChainArbitrum := false - var arbSys *precompilesgen.ArbSys codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) if err != nil { return nil, err } if len(codeAt) != 0 { isParentChainArbitrum = true - arbSys, err = precompilesgen.NewArbSys(types.ArbSysAddress, client) - if err != nil { - return nil, err + if arbSysPrecompile == nil { + return nil, errors.New("unable to create ArbSys from precompilesgen") } } return &HeaderReader{ client: client, config: config, isParentChainArbitrum: isParentChainArbitrum, - arbSys: arbSys, + arbSys: arbSysPrecompile, outChannels: make(map[chan<- *types.Header]struct{}), outChannelsBehind: make(map[chan<- *types.Header]struct{}), safe: cachedHeader{rpcBlockNum: big.NewInt(rpc.SafeBlockNumber.Int64())}, From 57f25acccc14d0633502136bacc4e637dc89ea02 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 1 Sep 2023 14:23:19 -0500 Subject: [PATCH 18/23] modify implementation to handle CodeAt errors in lb --- arbnode/node.go | 4 ++-- cmd/daserver/daserver.go | 2 +- cmd/nitro/nitro.go | 2 +- system_tests/das_test.go | 2 +- util/headerreader/header_reader.go | 20 +++++++++++--------- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index e3e9223b1d..7fde929771 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -237,7 +237,7 @@ func GenerateRollupConfig(prod bool, wasmModuleRoot common.Hash, rollupOwner com func DeployOnL1(ctx context.Context, l1client arbutil.L1Interface, deployAuth *bind.TransactOpts, batchPoster common.Address, authorizeValidators uint64, readerConfig headerreader.ConfigFetcher, config rollupgen.Config) (*chaininfo.RollupAddresses, error) { arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, readerConfig, arbSys) + l1Reader, err := headerreader.New(ctx, l1client, readerConfig, arbSys, true) if err != nil { return nil, err } @@ -614,7 +614,7 @@ func createNodeImpl( var l1Reader *headerreader.HeaderReader if config.ParentChainReader.Enable { arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys) + l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys, true) if err != nil { return nil, err } diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go index 335aba6a1b..6b874f4639 100644 --- a/cmd/daserver/daserver.go +++ b/cmd/daserver/daserver.go @@ -199,7 +199,7 @@ func startup() error { return err } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys) // TODO: config + l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys, true) // TODO: config if err != nil { return err } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index dd26fea46f..e404733b1e 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -357,7 +357,7 @@ func mainImpl() int { log.Crit("--node.validator.only-create-wallet-contract requires --node.validator.use-smart-contract-wallet") } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys) + l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys, true) if err != nil { log.Crit("failed to get L1 headerreader", "error", err) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 8889d2d53d..e79b993d03 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -235,7 +235,7 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { l1info, l1client, _, l1stack := createTestL1BlockChain(t, nil) defer requireClose(t, l1stack) arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index 8487ccd54b..c7fa937385 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -94,16 +94,18 @@ var TestConfig = Config{ UseFinalityData: false, } -func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface) (*HeaderReader, error) { +func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface, usePrecompilesgen bool) (*HeaderReader, error) { isParentChainArbitrum := false - codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) - if err != nil { - return nil, err - } - if len(codeAt) != 0 { - isParentChainArbitrum = true - if arbSysPrecompile == nil { - return nil, errors.New("unable to create ArbSys from precompilesgen") + if usePrecompilesgen { + codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) + if err != nil { + return nil, err + } + if len(codeAt) != 0 { + isParentChainArbitrum = true + if arbSysPrecompile == nil { + return nil, errors.New("unable to create ArbSys from precompilesgen") + } } } return &HeaderReader{ From 800c0a4d34fb49c139ef4d78d5cc2c8eec856bb0 Mon Sep 17 00:00:00 2001 From: Nodar Date: Mon, 4 Sep 2023 15:51:32 +0200 Subject: [PATCH 19/23] Run linters by specifying linter folder rather than go files --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 33487d0609..d6082261cd 100644 --- a/Makefile +++ b/Makefile @@ -304,8 +304,8 @@ contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(arbitrator_pro # strategic rules to minimize dependency building .make/lint: $(DEP_PREDICATE) build-node-deps $(ORDER_ONLY_PREDICATE) .make - go run linter/koanf/koanf.go linter/koanf/handlers.go ./... - go run linter/pointercheck/pointer.go ./... + go run ./linter/koanf ./... + go run ./linter/pointercheck ./... golangci-lint run --fix yarn --cwd contracts solhint @touch $@ From c2a6a54c923e22e4d6f027e9acd80b93507b0bbb Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Tue, 5 Sep 2023 11:07:52 -0500 Subject: [PATCH 20/23] address PR comments --- .../rediscoordinator/redis_coordinator.go | 34 +------------ .../seq-coordinator-manager.go | 48 +++++++++++-------- util/redisutil/redis_coordinator.go | 26 ++++++++++ 3 files changed, 57 insertions(+), 51 deletions(-) diff --git a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go index a393719a1d..782ab3801b 100644 --- a/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go +++ b/cmd/seq-coordinator-manager/rediscoordinator/redis_coordinator.go @@ -14,37 +14,6 @@ type RedisCoordinator struct { *redisutil.RedisCoordinator } -// GetPriorities returns the priority list of sequencers -func (rc *RedisCoordinator) GetPriorities(ctx context.Context) ([]string, map[string]int, error) { - prioritiesMap := make(map[string]int) - prioritiesString, err := rc.Client.Get(ctx, redisutil.PRIORITIES_KEY).Result() - if err != nil { - if errors.Is(err, redis.Nil) { - err = errors.New("sequencer priorities unset") - } - return []string{}, prioritiesMap, err - } - priorities := strings.Split(prioritiesString, ",") - for _, url := range priorities { - prioritiesMap[url]++ - } - return priorities, prioritiesMap, nil -} - -// GetLivelinessMap returns a map whose keys are sequencers that have their liveliness set to OK -func (rc *RedisCoordinator) GetLivelinessMap(ctx context.Context) (map[string]int, error) { - livelinessMap := make(map[string]int) - livelinessList, _, err := rc.Client.Scan(ctx, 0, redisutil.WANTS_LOCKOUT_KEY_PREFIX+"*", 0).Result() - if err != nil { - return livelinessMap, err - } - for _, elem := range livelinessList { - url := strings.TrimPrefix(elem, redisutil.WANTS_LOCKOUT_KEY_PREFIX) - livelinessMap[url]++ - } - return livelinessMap, nil -} - // UpdatePriorities updates the priority list of sequencers func (rc *RedisCoordinator) UpdatePriorities(ctx context.Context, priorities []string) error { prioritiesString := strings.Join(priorities, ",") @@ -53,6 +22,7 @@ func (rc *RedisCoordinator) UpdatePriorities(ctx context.Context, priorities []s if errors.Is(err, redis.Nil) { err = errors.New("sequencer priorities unset") } + return err } - return err + return nil } diff --git a/cmd/seq-coordinator-manager/seq-coordinator-manager.go b/cmd/seq-coordinator-manager/seq-coordinator-manager.go index 0a279cff64..a0123a9123 100644 --- a/cmd/seq-coordinator-manager/seq-coordinator-manager.go +++ b/cmd/seq-coordinator-manager/seq-coordinator-manager.go @@ -30,8 +30,8 @@ var nonPriorityForm = tview.NewForm() // Sequencer coordinator managment UI data store type manager struct { redisCoordinator *rediscoordinator.RedisCoordinator - prioritiesMap map[string]int - livelinessMap map[string]int + prioritiesSet map[string]bool + livelinessSet map[string]bool priorityList []string nonPriorityList []string } @@ -55,8 +55,8 @@ func main() { redisCoordinator: &rediscoordinator.RedisCoordinator{ RedisCoordinator: redisutilCoordinator, }, - prioritiesMap: make(map[string]int), - livelinessMap: make(map[string]int), + prioritiesSet: make(map[string]bool), + livelinessSet: make(map[string]bool), } seqManager.refreshAllLists(ctx) @@ -92,7 +92,7 @@ func main() { }) priorityForm.AddButton("Remove", func() { url := seqManager.priorityList[index] - delete(seqManager.prioritiesMap, url) + delete(seqManager.prioritiesSet, url) seqManager.updatePriorityList(ctx, index, 0) seqManager.priorityList = seqManager.priorityList[1:] @@ -122,7 +122,7 @@ func main() { nonPriorityForm.AddButton("Update", func() { key := seqManager.nonPriorityList[index] seqManager.priorityList = append(seqManager.priorityList, key) - seqManager.prioritiesMap[key]++ + seqManager.prioritiesSet[key] = true index = len(seqManager.priorityList) - 1 seqManager.updatePriorityList(ctx, index, target) @@ -188,9 +188,11 @@ func main() { seqManager.addSeqPriorityForm(ctx) pages.SwitchToPage("Add Sequencer") } else if event.Rune() == 99 { - if prioritySeqList.HasFocus() { + if prioritySeqList.HasFocus() || priorityForm.HasFocus() { + priorityForm.Clear(true) app.SetFocus(nonPrioritySeqList) } else { + nonPriorityForm.Clear(true) app.SetFocus(prioritySeqList) } } else if event.Rune() == 113 { @@ -217,8 +219,8 @@ func (sm *manager) updatePriorityList(ctx context.Context, index int, target int } urlList := []string{} - for url := range sm.livelinessMap { - if _, ok := sm.prioritiesMap[url]; !ok { + for url := range sm.livelinessSet { + if _, ok := sm.prioritiesSet[url]; !ok { urlList = append(urlList, url) } } @@ -238,7 +240,7 @@ func (sm *manager) populateLists(ctx context.Context) { sec = fmt.Sprintf(" %vchosen", emoji.LeftArrow) } status := fmt.Sprintf("(%d) %v ", index, emoji.RedCircle) - if _, ok := sm.livelinessMap[seqURL]; ok { + if _, ok := sm.livelinessSet[seqURL]; ok { status = fmt.Sprintf("(%d) %v ", index, emoji.GreenCircle) } prioritySeqList.AddItem(status+seqURL+sec, "", rune(0), nil).SetSecondaryTextColor(tcell.ColorPurple) @@ -264,8 +266,8 @@ func (sm *manager) addSeqPriorityForm(ctx context.Context) *tview.Form { }) addSeqForm.AddButton("Add", func() { // check if url is valid, i.e it doesnt already exist in the priority list - if _, ok := sm.prioritiesMap[URL]; !ok && URL != "" { - sm.prioritiesMap[URL]++ + if _, ok := sm.prioritiesSet[URL]; !ok && URL != "" { + sm.prioritiesSet[URL] = true sm.priorityList = append(sm.priorityList, URL) } sm.populateLists(ctx) @@ -285,24 +287,32 @@ func (sm *manager) pushUpdates(ctx context.Context) { // refreshAllLists gets the current status of all the lists displayed in the UI func (sm *manager) refreshAllLists(ctx context.Context) { - sequencerURLList, mapping, err := sm.redisCoordinator.GetPriorities(ctx) + priorityList, err := sm.redisCoordinator.GetPriorities(ctx) if err != nil { panic(err) } - sm.priorityList = sequencerURLList - sm.prioritiesMap = mapping + sm.priorityList = priorityList + sm.prioritiesSet = getMapfromlist(priorityList) - mapping, err = sm.redisCoordinator.GetLivelinessMap(ctx) + livelinessList, err := sm.redisCoordinator.GetLiveliness(ctx) if err != nil { panic(err) } - sm.livelinessMap = mapping + sm.livelinessSet = getMapfromlist(livelinessList) urlList := []string{} - for url := range sm.livelinessMap { - if _, ok := sm.prioritiesMap[url]; !ok { + for url := range sm.livelinessSet { + if _, ok := sm.prioritiesSet[url]; !ok { urlList = append(urlList, url) } } sm.nonPriorityList = urlList } + +func getMapfromlist(list []string) map[string]bool { + mapping := make(map[string]bool) + for _, url := range list { + mapping[url] = true + } + return mapping +} diff --git a/util/redisutil/redis_coordinator.go b/util/redisutil/redis_coordinator.go index 0ee92fef17..357dfb2e93 100644 --- a/util/redisutil/redis_coordinator.go +++ b/util/redisutil/redis_coordinator.go @@ -76,6 +76,32 @@ func (c *RedisCoordinator) CurrentChosenSequencer(ctx context.Context) (string, return current, nil } +// GetPriorities returns the priority list of sequencers +func (rc *RedisCoordinator) GetPriorities(ctx context.Context) ([]string, error) { + prioritiesString, err := rc.Client.Get(ctx, PRIORITIES_KEY).Result() + if err != nil { + if errors.Is(err, redis.Nil) { + err = errors.New("sequencer priorities unset") + } + return []string{}, err + } + prioritiesList := strings.Split(prioritiesString, ",") + return prioritiesList, nil +} + +// GetLiveliness returns a map whose keys are sequencers that have their liveliness set to OK +func (rc *RedisCoordinator) GetLiveliness(ctx context.Context) ([]string, error) { + livelinessList, _, err := rc.Client.Scan(ctx, 0, WANTS_LOCKOUT_KEY_PREFIX+"*", 0).Result() + if err != nil { + return []string{}, err + } + for i, elem := range livelinessList { + url := strings.TrimPrefix(elem, WANTS_LOCKOUT_KEY_PREFIX) + livelinessList[i] = url + } + return livelinessList, nil +} + func MessageKeyFor(pos arbutil.MessageIndex) string { return fmt.Sprintf("%s%d", MESSAGE_KEY_PREFIX, pos) } From 176d7be4df2e227799355451b56c969aaff67b60 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 13 Sep 2023 11:01:32 -0600 Subject: [PATCH 21/23] Make batch revert nextToCheck a field --- arbnode/batch_poster.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 8144c9b7be..89a36eba91 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -76,7 +76,8 @@ type BatchPoster struct { backlog uint64 lastHitL1Bounds time.Time // The last time we wanted to post a message but hit the L1 bounds - batchReverted atomic.Bool // indicates whether data poster batch was reverted + batchReverted atomic.Bool // indicates whether data poster batch was reverted + nextRevertCheckBlock int64 // the last parent block scanned for reverting batches } type l1BlockBound int @@ -263,13 +264,12 @@ func NewBatchPoster(dataPosterDB ethdb.Database, l1Reader *headerreader.HeaderRe // contain reverted batch_poster transaction. // It returns true if it finds batch posting needs to halt, which is true if a batch reverts // unless the data poster is configured with noop storage which can tolerate reverts. -// From must be a pointer to the starting block, which is updated after each block is checked for reverts -func (b *BatchPoster) checkReverts(ctx context.Context, from *int64, to int64) (bool, error) { - if *from > to { - return false, fmt.Errorf("wrong range, from: %d > to: %d", from, to) +func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) { + if b.nextRevertCheckBlock > to { + return false, fmt.Errorf("wrong range, from: %d > to: %d", b.nextRevertCheckBlock, to) } - for ; *from <= to; *from++ { - number := big.NewInt(*from) + for ; b.nextRevertCheckBlock <= to; b.nextRevertCheckBlock++ { + number := big.NewInt(b.nextRevertCheckBlock) block, err := b.l1Reader.Client().BlockByNumber(ctx, number) if err != nil { return false, fmt.Errorf("getting block: %v by number: %w", number, err) @@ -305,7 +305,6 @@ func (b *BatchPoster) pollForReverts(ctx context.Context) { headerCh, unsubscribe := b.l1Reader.Subscribe(false) defer unsubscribe() - nextToCheck := int64(0) // the first unchecked block for { // Poll until: // - L1 headers reader channel is closed, or @@ -317,19 +316,20 @@ func (b *BatchPoster) pollForReverts(ctx context.Context) { log.Info("L1 headers channel checking for batch poster reverts has been closed") return } + blockNum := h.Number.Int64() // If this is the first block header, set last seen as number-1. // We may see same block number again if there is L1 reorg, in that // case we check the block again. - if nextToCheck == 0 || nextToCheck == h.Number.Int64() { - nextToCheck = h.Number.Int64() + if b.nextRevertCheckBlock == 0 || b.nextRevertCheckBlock > blockNum { + b.nextRevertCheckBlock = blockNum } - if h.Number.Int64()-nextToCheck > 100 { - log.Warn("Large gap between last seen and current block number, skipping check for reverts", "last", nextToCheck, "current", h.Number) - nextToCheck = h.Number.Int64() + if blockNum-b.nextRevertCheckBlock > 100 { + log.Warn("Large gap between last seen and current block number, skipping check for reverts", "last", b.nextRevertCheckBlock, "current", blockNum) + b.nextRevertCheckBlock = blockNum continue } - reverted, err := b.checkReverts(ctx, &nextToCheck, h.Number.Int64()) + reverted, err := b.checkReverts(ctx, blockNum) if err != nil { logLevel := log.Error if strings.Contains(err.Error(), "not found") { From bf0c01a1e98a846801288d1893f3a171ff96cb35 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 13 Sep 2023 15:27:04 -0500 Subject: [PATCH 22/23] fix failing test --- cmd/deploy/deploy.go | 5 ++++- system_tests/common_test.go | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index 17725a7a4c..3e698749d5 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -14,10 +14,12 @@ import ( "github.com/offchainlabs/nitro/cmd/chaininfo" "github.com/offchainlabs/nitro/cmd/genericconf" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/validator/server_common" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" @@ -127,7 +129,8 @@ func main() { panic(fmt.Errorf("failed to deserialize chain config: %w", err)) } - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }, arbSys, true) if err != nil { panic(fmt.Errorf("failed to create header reader: %w", err)) } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index b92fbf7578..a643b1b719 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -476,7 +476,8 @@ func DeployOnTestL1( serializedChainConfig, err := json.Marshal(chainConfig) Require(t, err) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() From 6a9fd710cd6611ec6adcdadc56f8307afe13ecce Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 13 Sep 2023 19:42:33 -0500 Subject: [PATCH 23/23] revert impl --- arbnode/node.go | 2 +- cmd/daserver/daserver.go | 2 +- cmd/deploy/deploy.go | 2 +- cmd/nitro/nitro.go | 2 +- system_tests/common_test.go | 2 +- system_tests/das_test.go | 2 +- util/headerreader/header_reader.go | 11 +++++------ 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index a40c84fa94..5bdc716264 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -606,7 +606,7 @@ func createNodeImpl( var l1Reader *headerreader.HeaderReader if config.ParentChainReader.Enable { arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys, true) + l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys) if err != nil { return nil, err } diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go index 6b874f4639..335aba6a1b 100644 --- a/cmd/daserver/daserver.go +++ b/cmd/daserver/daserver.go @@ -199,7 +199,7 @@ func startup() error { return err } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys, true) // TODO: config + l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys) // TODO: config if err != nil { return err } diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index 3e698749d5..d687821e8b 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -130,7 +130,7 @@ func main() { } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }, arbSys) if err != nil { panic(fmt.Errorf("failed to create header reader: %w", err)) } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index f2cb3a37d2..a7dc7f26f9 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -357,7 +357,7 @@ func mainImpl() int { log.Crit("--node.validator.only-create-wallet-contract requires --node.validator.use-smart-contract-wallet") } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys) if err != nil { log.Crit("failed to get L1 headerreader", "error", err) } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index a643b1b719..9fd002bd94 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -477,7 +477,7 @@ func DeployOnTestL1( Require(t, err) arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/system_tests/das_test.go b/system_tests/das_test.go index e79b993d03..8889d2d53d 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -235,7 +235,7 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { l1info, l1client, _, l1stack := createTestL1BlockChain(t, nil) defer requireClose(t, l1stack) arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index c7fa937385..befd54ace3 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -94,25 +94,24 @@ var TestConfig = Config{ UseFinalityData: false, } -func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface, usePrecompilesgen bool) (*HeaderReader, error) { +func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface) (*HeaderReader, error) { isParentChainArbitrum := false - if usePrecompilesgen { + var arbSys ArbSysInterface + if arbSysPrecompile != nil { codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) if err != nil { return nil, err } if len(codeAt) != 0 { isParentChainArbitrum = true - if arbSysPrecompile == nil { - return nil, errors.New("unable to create ArbSys from precompilesgen") - } + arbSys = arbSysPrecompile } } return &HeaderReader{ client: client, config: config, isParentChainArbitrum: isParentChainArbitrum, - arbSys: arbSysPrecompile, + arbSys: arbSys, outChannels: make(map[chan<- *types.Header]struct{}), outChannelsBehind: make(map[chan<- *types.Header]struct{}), safe: cachedHeader{rpcBlockNum: big.NewInt(rpc.SafeBlockNumber.Int64())},