Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app/obolapi: specify HTTP requests timeout #3098

Merged
merged 3 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions app/obolapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,43 @@ import (
const (
// launchpadReturnPathFmt is the URL path format string at which one can find details for a given cluster lock hash.
launchpadReturnPathFmt = "/lock/0x%X/launchpad"

// defaultTimeout is the default HTTP request timeout if not specified
defaultTimeout = 10 * time.Second
gsora marked this conversation as resolved.
Show resolved Hide resolved
)

// New returns a new Client.
func New(urlStr string) (Client, error) {
func New(urlStr string, options ...func(*Client)) (Client, error) {
_, err := url.ParseRequestURI(urlStr) // check that urlStr is valid
if err != nil {
return Client{}, errors.Wrap(err, "could not parse Obol API URL")
}

return Client{
// always set a default timeout, even if no options are provided
options = append([]func(*Client){WithTimeout(defaultTimeout)}, options...)

cl := Client{
baseURL: urlStr,
}, nil
}

for _, opt := range options {
opt(&cl)
}

return cl, nil
}

// Client is the REST client for obol-api requests.
type Client struct {
baseURL string // Base obol-api URL
baseURL string // Base obol-api URL
reqTimeout time.Duration // Timeout to use for HTTP requests
}

// WithTimeout sets the HTTP request timeout for all Client calls to the provided value.
func WithTimeout(timeout time.Duration) func(*Client) {
return func(client *Client) {
client.reqTimeout = timeout
}
}

// url returns a *url.URL from the baseURL stored in c.
Expand All @@ -49,11 +69,9 @@ func (c Client) url() *url.URL {
return baseURL
}

// PublishLock posts the lockfile to obol-api. It has a 30s timeout.
// PublishLock posts the lockfile to obol-api.
// It respects the timeout specified in the Client instance.
func (c Client) PublishLock(ctx context.Context, lock cluster.Lock) error {
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()

addr := c.url()
addr.Path = "lock"

Expand All @@ -62,6 +80,9 @@ func (c Client) PublishLock(ctx context.Context, lock cluster.Lock) error {
return errors.Wrap(err, "marshal lock")
}

ctx, cancel := context.WithTimeout(ctx, c.reqTimeout)
defer cancel()

err = httpPost(ctx, addr, b)
if err != nil {
return err
Expand Down
23 changes: 23 additions & 0 deletions app/obolapi/api_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright © 2022-2024 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1

package obolapi

import (
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestWithTimeout(t *testing.T) {
// no timeout = 10s timeout
oapi, err := New("http://url.com")
require.NoError(t, err)
require.Equal(t, defaultTimeout, oapi.reqTimeout)

// with timeout = timeout specified
timeout := 1 * time.Minute
oapi, err = New("http://url.com", WithTimeout(timeout))
require.NoError(t, err)
require.Equal(t, timeout, oapi.reqTimeout)
}
8 changes: 8 additions & 0 deletions app/obolapi/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func fullExitURL(valPubkey, lockHash string, shareIndex uint64) string {
}

// PostPartialExit POSTs the set of msg's to the Obol API, for a given lock hash.
// It respects the timeout specified in the Client instance.
func (c Client) PostPartialExit(ctx context.Context, lockHash []byte, shareIndex uint64, identityKey *k1.PrivateKey, exitBlobs ...ExitBlob) error {
lockHashStr := "0x" + hex.EncodeToString(lockHash)

Expand Down Expand Up @@ -103,6 +104,9 @@ func (c Client) PostPartialExit(ctx context.Context, lockHash []byte, shareIndex
return errors.Wrap(err, "json marshal error")
}

ctx, cancel := context.WithTimeout(ctx, c.reqTimeout)
defer cancel()

req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), bytes.NewReader(data))
if err != nil {
return errors.Wrap(err, "http new post request")
Expand All @@ -127,6 +131,7 @@ func (c Client) PostPartialExit(ctx context.Context, lockHash []byte, shareIndex
}

// GetFullExit gets the full exit message for a given validator public key, lock hash and share index.
// It respects the timeout specified in the Client instance.
func (c Client) GetFullExit(ctx context.Context, valPubkey string, lockHash []byte, shareIndex uint64, identityKey *k1.PrivateKey) (ExitBlob, error) {
valPubkeyBytes, err := from0x(valPubkey, 48) // public key is 48 bytes long
if err != nil {
Expand All @@ -142,6 +147,9 @@ func (c Client) GetFullExit(ctx context.Context, valPubkey string, lockHash []by

u.Path = path

ctx, cancel := context.WithTimeout(ctx, c.reqTimeout)
defer cancel()

req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
if err != nil {
return ExitBlob{}, errors.Wrap(err, "http new get request")
Expand Down
1 change: 1 addition & 0 deletions cmd/dkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@

func bindPublishFlags(flags *pflag.FlagSet, config *dkg.Config) {
flags.StringVar(&config.PublishAddr, "publish-address", "https://api.obol.tech", "The URL to publish the cluster to.")
flags.DurationVar(&config.PublishTimeout, "publish-timeout", 30*time.Second, "Timeout for publishing a cluster, consider increasing if the cluster contains more than 200 validators.")

Check warning on line 67 in cmd/dkg.go

View check run for this annotation

Codecov / codecov/patch

cmd/dkg.go#L67

Added line #L67 was not covered by tests
flags.BoolVar(&config.Publish, "publish", false, "Publish the created cluster to a remote API.")
}

Expand Down
6 changes: 6 additions & 0 deletions cmd/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
ValidatorKeysDir string
LockFilePath string
PublishAddress string
PublishTimeout time.Duration
ExitEpoch uint64
FetchedExitPath string
PlaintextOutput bool
Expand Down Expand Up @@ -56,6 +57,7 @@
exitEpoch
exitFromFile
beaconNodeTimeout
publishTimeout
)

func (ef exitFlag) String() string {
Expand All @@ -78,6 +80,8 @@
return "exit-from-file"
case beaconNodeTimeout:
return "beacon-node-timeout"
case publishTimeout:
return "publish-timeout"

Check warning on line 84 in cmd/exit.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit.go#L83-L84

Added lines #L83 - L84 were not covered by tests
default:
return "unknown"
}
Expand Down Expand Up @@ -119,6 +123,8 @@
cmd.Flags().StringVar(&config.ExitFromFilePath, exitFromFile.String(), "", maybeRequired("Retrieves a signed exit message from a pre-prepared file instead of --publish-address."))
case beaconNodeTimeout:
cmd.Flags().DurationVar(&config.BeaconNodeTimeout, beaconNodeTimeout.String(), 30*time.Second, maybeRequired("Timeout for beacon node HTTP calls."))
case publishTimeout:
cmd.Flags().DurationVar(&config.PublishTimeout, publishTimeout.String(), 30*time.Second, "Timeout for publishing a signed exit to the publish-address API.")

Check warning on line 127 in cmd/exit.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit.go#L126-L127

Added lines #L126 - L127 were not covered by tests
}

if f.required {
Expand Down
7 changes: 4 additions & 3 deletions cmd/exit_broadcast.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"os"
"strings"
"time"

eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0"
k1 "github.com/decred/dcrd/dcrec/secp256k1/v4"
Expand Down Expand Up @@ -95,7 +96,7 @@ func runBcastFullExit(ctx context.Context, config exitConfig) error {
fullExit, err = exitFromPath(maybeExitFilePath)
} else {
log.Info(ctx, "Retrieving full exit message from publish address")
fullExit, err = exitFromObolAPI(ctx, config.ValidatorPubkey, config.PublishAddress, cl, identityKey)
fullExit, err = exitFromObolAPI(ctx, config.ValidatorPubkey, config.PublishAddress, config.PublishTimeout, cl, identityKey)
}

if err != nil {
Expand Down Expand Up @@ -141,8 +142,8 @@ func runBcastFullExit(ctx context.Context, config exitConfig) error {
}

// exitFromObolAPI fetches an eth2p0.SignedVoluntaryExit message from publishAddr for the given validatorPubkey.
func exitFromObolAPI(ctx context.Context, validatorPubkey, publishAddr string, cl *manifestpb.Cluster, identityKey *k1.PrivateKey) (eth2p0.SignedVoluntaryExit, error) {
oAPI, err := obolapi.New(publishAddr)
func exitFromObolAPI(ctx context.Context, validatorPubkey, publishAddr string, publishTimeout time.Duration, cl *manifestpb.Cluster, identityKey *k1.PrivateKey) (eth2p0.SignedVoluntaryExit, error) {
oAPI, err := obolapi.New(publishAddr, obolapi.WithTimeout(publishTimeout))
if err != nil {
return eth2p0.SignedVoluntaryExit{}, errors.Wrap(err, "could not create obol api client")
}
Expand Down
5 changes: 4 additions & 1 deletion cmd/exit_broadcast_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func testRunBcastFullExitCmdFlow(t *testing.T, fromFile bool) {
PublishAddress: srv.URL,
ExitEpoch: 194048,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
}

require.NoError(t, runSignPartialExit(ctx, config), "operator index: %v", idx)
Expand All @@ -139,10 +140,11 @@ func testRunBcastFullExitCmdFlow(t *testing.T, fromFile bool) {
PublishAddress: srv.URL,
ExitEpoch: 194048,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
}

if fromFile {
exit, err := exitFromObolAPI(ctx, lock.Validators[0].PublicKeyHex(), srv.URL, cl, enrs[0])
exit, err := exitFromObolAPI(ctx, lock.Validators[0].PublicKeyHex(), srv.URL, 10*time.Second, cl, enrs[0])
require.NoError(t, err)

exitBytes, err := json.Marshal(exit)
Expand Down Expand Up @@ -289,6 +291,7 @@ func Test_runBcastFullExitCmd_Config(t *testing.T) {
PublishAddress: oapiURL,
ExitEpoch: 0,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
}

if test.badExistingExitPath {
Expand Down
2 changes: 1 addition & 1 deletion cmd/exit_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func runFetchExit(ctx context.Context, config exitConfig) error {

ctx = log.WithCtx(ctx, z.Str("validator", validator.String()))

oAPI, err := obolapi.New(config.PublishAddress)
oAPI, err := obolapi.New(config.PublishAddress, obolapi.WithTimeout(config.PublishTimeout))
if err != nil {
return errors.Wrap(err, "could not create obol api client")
}
Expand Down
2 changes: 2 additions & 0 deletions cmd/exit_fetch_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func Test_runFetchExitFullFlow(t *testing.T) {
PublishAddress: srv.URL,
ExitEpoch: 194048,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
}

require.NoError(t, runSignPartialExit(ctx, config), "operator index: %v", idx)
Expand All @@ -120,6 +121,7 @@ func Test_runFetchExitFullFlow(t *testing.T) {
LockFilePath: filepath.Join(baseDir, "cluster-lock.json"),
PublishAddress: srv.URL,
FetchedExitPath: root,
PublishTimeout: 10 * time.Second,
}

require.NoError(t, runFetchExit(ctx, config))
Expand Down
2 changes: 1 addition & 1 deletion cmd/exit_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func runSignPartialExit(ctx context.Context, config exitConfig) error {

eth2Cl.SetForkVersion([4]byte(cl.GetForkVersion()))

oAPI, err := obolapi.New(config.PublishAddress)
oAPI, err := obolapi.New(config.PublishAddress, obolapi.WithTimeout(config.PublishTimeout))
if err != nil {
return errors.Wrap(err, "could not create obol api client")
}
Expand Down
2 changes: 2 additions & 0 deletions cmd/exit_sign_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func Test_runSubmitPartialExitFlow(t *testing.T) {
PublishAddress: srv.URL,
ExitEpoch: 194048,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
}

require.NoError(t, runSignPartialExit(ctx, config))
Expand Down Expand Up @@ -273,6 +274,7 @@ func Test_runSubmitPartialExit_Config(t *testing.T) {
PublishAddress: oapiURL,
ExitEpoch: 0,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
}

require.ErrorContains(t, runSignPartialExit(ctx, config), test.errData)
Expand Down
11 changes: 6 additions & 5 deletions dkg/dkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ type Config struct {
KeymanagerAddr string
KeymanagerAuthToken string

PublishAddr string
Publish bool
PublishAddr string
PublishTimeout time.Duration
Publish bool

TestConfig TestConfig
}
Expand Down Expand Up @@ -334,7 +335,7 @@ func Run(ctx context.Context, conf Config) (err error) {
var dashboardURL string

if conf.Publish {
if dashboardURL, err = writeLockToAPI(ctx, conf.PublishAddr, lock); err != nil {
if dashboardURL, err = writeLockToAPI(ctx, conf.PublishAddr, lock, conf.PublishTimeout); err != nil {
log.Warn(ctx, "Couldn't publish lock file to Obol API", err)
}
}
Expand Down Expand Up @@ -1059,8 +1060,8 @@ func createDistValidators(shares []share, depositDatas [][]eth2p0.DepositData, v
}

// writeLockToAPI posts the lock file to obol-api and returns the Launchpad dashboard URL.
func writeLockToAPI(ctx context.Context, publishAddr string, lock cluster.Lock) (string, error) {
cl, err := obolapi.New(publishAddr)
func writeLockToAPI(ctx context.Context, publishAddr string, lock cluster.Lock, timeout time.Duration) (string, error) {
cl, err := obolapi.New(publishAddr, obolapi.WithTimeout(timeout))
if err != nil {
return "", err
}
Expand Down
3 changes: 2 additions & 1 deletion dkg/dkg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ func testDKG(t *testing.T, def cluster.Definition, dir string, p2pKeys []*k1.Pri
},
SyncOpts: []func(*dkgsync.Client){dkgsync.WithPeriod(time.Millisecond * 50)},
},
ShutdownDelay: 1 * time.Second,
ShutdownDelay: 1 * time.Second,
PublishTimeout: 30 * time.Second,
}

allReceivedKeystores := make(chan struct{}) // Receives struct{} for each `numNodes` keystore intercepted by the keymanager server
Expand Down
Loading