Skip to content

Commit

Permalink
app/obolapi: specify HTTP requests timeout
Browse files Browse the repository at this point in the history
Allow users of `app/obolapi` to specify HTTP requests timeout.

As a result of this feature, add the `--publish-timeout` CLI flag to `exit` and `dkg` commands.
  • Loading branch information
gsora committed May 23, 2024
1 parent 9daf1c8 commit 7b2de52
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 20 deletions.
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
)

// 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 bindDataDirFlag(flags *pflag.FlagSet, dataDir *string) {

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, "Publish API timeout, increase if the cluster being created contains many 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 @@ type exitConfig struct {
ValidatorKeysDir string
LockFilePath string
PublishAddress string
PublishTimeout time.Duration
ExitEpoch uint64
FetchedExitPath string
PlaintextOutput bool
Expand Down Expand Up @@ -56,6 +57,7 @@ const (
exitEpoch
exitFromFile
beaconNodeTimeout
publishTimeout
)

func (ef exitFlag) String() string {
Expand All @@ -78,6 +80,8 @@ func (ef exitFlag) String() string {
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 @@ func bindExitFlags(cmd *cobra.Command, config *exitConfig, flags []exitCLIFlag)
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, "Publish API timeout, increase if API interactions time out.")

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

0 comments on commit 7b2de52

Please sign in to comment.