Merge pull request #51 from smartcontractkit/ethclient-refactor

Refactor the main timelock loop

.tool-versions

@@ -1,2 +1,2 @@
-golang 1.20
-golangci-lint 1.52.2
+golang 1.22
+golangci-lint 1.57.2

Makefile

@@ -35,6 +35,11 @@ test:
 	@echo "\n\t$(C_GREEN)# Run test and generate new coverage.out$(C_END)"
 	go test -short -coverprofile=coverage.out -covermode=atomic -race ./...
 
+.PHONY: coverage
+coverage:
+	@echo "\n\t$(C_GREEN)# View coverage.out report$(C_END)"
+	@go tool cover -html="coverage.out"
+
 .PHONY: build
 build: clean
 	@echo "\n\t$(C_GREEN)# Build binary $(BINARY)$(C_END)"

cmd/start.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"context"
 	"math/big"
 
 	"github.com/smartcontractkit/timelock-worker/pkg/cli"
@@ -44,15 +43,12 @@ func startCommand() *cobra.Command {
 }
 
 func startHandler(cmd *cobra.Command, _ []string) {
-	// Use this ctx as the base context.
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
 	go startHTTPHealthServer()
 	go startMetricsServer()
-	startTimelock(ctx, cmd)
+	startTimelock(cmd)
 }
 
-func startTimelock(ctx context.Context, cmd *cobra.Command) {
+func startTimelock(cmd *cobra.Command) {
 	nodeURL, err := cmd.Flags().GetString("node-url")
 	if err != nil {
 		logs.Fatal().Msgf("value of node-url not set: %s", err.Error())
@@ -88,7 +84,7 @@ func startTimelock(ctx context.Context, cmd *cobra.Command) {
 		logs.Fatal().Msgf("error creating the timelock-worker: %s", err.Error())
 	}
 
-	if err := tWorker.Listen(ctx); err != nil {
+	if err := tWorker.Listen(); err != nil {
 		logs.Fatal().Msgf("error while starting timelock-worker: %s", err.Error())
 	}
 

go.mod

@@ -1,6 +1,6 @@
 module github.com/smartcontractkit/timelock-worker
 
-go 1.20
+go 1.22
 
 require (
 	github.com/ethereum/go-ethereum v1.13.15

pkg/cli/config_test.go

@@ -13,15 +13,17 @@ import (
 
 func TestNewConfigRaw(t *testing.T) {
 	var newConfig = &cli.Config{
-		NodeURL:         "foo:test",
-		TimelockAddress: "0x12345",
-		PrivateKey:      "0123456789",
-		FromBlock:       0,
+		NodeURL:          "foo:test",
+		TimelockAddress:  "0x12345",
+		CallProxyAddress: "0x12345",
+		PrivateKey:       "0123456789",
+		FromBlock:        0,
 	}
 
 	if assert.NotNil(t, newConfig) {
 		assert.Equal(t, "foo:test", newConfig.NodeURL)
 		assert.Equal(t, "0x12345", newConfig.TimelockAddress)
+		assert.Equal(t, "0x12345", newConfig.CallProxyAddress)
 		assert.Equal(t, "0123456789", newConfig.PrivateKey)
 		assert.Equal(t, int64(0), newConfig.FromBlock)
 	}
@@ -37,14 +39,16 @@ func TestNewTimelockCLIFromEnvVar(t *testing.T) {
 
 	t.Setenv("NODE_URL", "wss://goerli/test")
 	t.Setenv("TIMELOCK_ADDRESS", "0x12345")
+	t.Setenv("CALL_PROXY_ADDRESS", "0x12345")
 	t.Setenv("PRIVATE_KEY", "1234567890")
 	t.Setenv("FROM_BLOCK", "1234567890")
 
 	var wantedConfig = cli.Config{
-		NodeURL:         "wss://goerli/test",
-		TimelockAddress: "0x12345",
-		PrivateKey:      "1234567890",
-		FromBlock:       1234567890,
+		NodeURL:          "wss://goerli/test",
+		TimelockAddress:  "0x12345",
+		CallProxyAddress: "0x12345",
+		PrivateKey:       "1234567890",
+		FromBlock:        1234567890,
 	}
 
 	tests := []struct {

pkg/isclosed/isclosed.go

@@ -0,0 +1,37 @@
+package isclosed
+
+import (
+	"context"
+	"sync"
+)
+
+// All returns a channel that is closed either when each of the channels is read from or the passed
+// context is canceled.  All is useful for implementing graceful shutdown of a number of Sends or
+// other running goroutines that indicate their state via a returned read only channel.  The graceful
+// shutdown can be circumvented via the context passed to All to ensure shutdowns will not deadlock.
+func All(ctx context.Context, done ...<-chan struct{}) <-chan struct{} {
+	var (
+		shutdown = make(chan struct{})
+		wg       sync.WaitGroup
+	)
+
+	wg.Add(len(done))
+	for _, ch := range done {
+		go func() {
+			defer wg.Done()
+
+			select {
+			case <-ctx.Done():
+			case <-ch:
+			}
+		}()
+	}
+
+	go func() {
+		defer close(shutdown)
+
+		wg.Wait()
+	}()
+
+	return shutdown
+}

pkg/isclosed/isclosed_test.go

@@ -0,0 +1,96 @@
+package isclosed
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+var maxTestTimeout = 3 * time.Second
+
+func TestAll_DoneAfterAllClose(t *testing.T) {
+	var (
+		ctx     = context.Background()
+		a, b, c = make(chan struct{}), make(chan struct{}), make(chan struct{})
+		done    = All(ctx, a, b, c)
+	)
+
+	close(a)
+	close(b)
+	close(c) // close all channels
+	eventually(t, done, maxTestTimeout)
+}
+
+func TestAll_DoneAfterCtxCancel(t *testing.T) {
+	var (
+		ctx, cancel = context.WithCancel(context.Background())
+		a, b, c     = make(chan struct{}), make(chan struct{}), make(chan struct{})
+		done        = All(ctx, a, b, c)
+	)
+
+	close(a)
+	close(b)
+	cancel() // c is never closed, but context is canceled
+	eventually(t, done, maxTestTimeout)
+}
+
+func TestAll_DoneAfterCtxCancelWithNilChannels(t *testing.T) {
+	var (
+		ctx, cancel = context.WithCancel(context.Background())
+		done        = All(ctx, nil, nil, nil)
+	)
+
+	cancel()
+	eventually(t, done, maxTestTimeout)
+}
+
+// TestAll_DoneNonBlocking verifies that if all the input channels close, the All function's returned
+// channel should also close regardless of the order of the input channel arguments.
+func TestAll_DoneNonBlocking(t *testing.T) {
+	var (
+		ctx     = context.Background()
+		a, b, c = make(chan struct{}), make(chan struct{}), make(chan struct{})
+		start   = make(chan struct{})
+
+		// done is only closed once all three input channels are closed as the context is never
+		// cancelled.
+		done = All(ctx, c, b, a)
+	)
+
+	// By default channel a closes with no dependencies.
+	go func() {
+		<-start
+		close(a)
+	}()
+
+	// Only close channel b after channel a is closed.
+	go func() {
+		<-a
+		close(b)
+	}()
+
+	// Only close the c channel after both channel b and channel a are closed.
+	go func() {
+		<-a
+		<-b
+		close(c)
+	}()
+
+	// Start the closing of the channels once all waiting routines are running.
+	close(start)
+
+	// Require that the done channel is eventually closed without context cancellation even with
+	// dependencies on closing between various channels as long as there is no deadlock state.
+	eventually(t, done, maxTestTimeout)
+}
+
+// eventually blocks until done is closed or d time duration passes.
+func eventually(t *testing.T, done <-chan struct{}, d time.Duration) {
+	t.Helper()
+
+	select {
+	case <-done:
+	case <-time.After(d):
+		t.Fatal("timed out waiting for done to close")
+	}
+}

pkg/timelock/const.go

@@ -4,6 +4,7 @@ import "time"
 
 const (
 	defaultSchedulerDelay time.Duration = 15 * time.Minute
+	maxSubRetries         int           = 5
 
 	eventCallScheduled  string = "CallScheduled"
 	eventCallExecuted   string = "CallExecuted"