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

feat: Update wait interface, Add FetchStakingOperation and ReloadStakingOperation #37

Merged
merged 1 commit into from
Sep 24, 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

## Unreleased

## [0.0.11] - 2024-09-23

- Fix a bug where correlation id returned from backend wasn't being relayed to the user
- Add FetchStakingOperation to fetch a staking operation by networkID, addressID and stakingOperationID
- Add ReloadStakingOperation to reload a given staking operation with latest data from the backend

## [0.0.10] - 2024-09-20

Expand Down
3 changes: 1 addition & 2 deletions examples/ethereum/dedicated-eth-stake/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ func main() {
log.Fatalf("error building staking operation: %v", err)
}

stakeOperation, err = client.Wait(ctx, stakeOperation, coinbase.WithWaitTimeoutSeconds(600))
if err != nil {
if err := client.Wait(ctx, stakeOperation, coinbase.WithWaitTimeoutSeconds(600)); err != nil {
log.Fatalf("error waiting for staking operation: %v", err)
}

Expand Down
2 changes: 1 addition & 1 deletion examples/solana/claim-stake/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func main() {

log.Printf("Staking operation ID: %s\n\n", stakingOperation.ID())

stakingOperation, err = client.Wait(ctx, stakingOperation, coinbase.WithWaitTimeoutSeconds(60))
err = client.Wait(ctx, stakingOperation, coinbase.WithWaitTimeoutSeconds(60))
if err != nil {
log.Fatalf("error waiting for staking operation: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion examples/solana/stake/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func main() {

log.Printf("Staking operation ID: %s\n\n", stakingOperation.ID())

stakingOperation, err = client.Wait(ctx, stakingOperation, coinbase.WithWaitTimeoutSeconds(60))
err = client.Wait(ctx, stakingOperation, coinbase.WithWaitTimeoutSeconds(60))
if err != nil {
log.Fatalf("error waiting for staking operation: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion examples/solana/unstake/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func main() {

log.Printf("Staking operation ID: %s\n\n", stakingOperation.ID())

stakingOperation, err = client.Wait(ctx, stakingOperation, coinbase.WithWaitTimeoutSeconds(60))
err = client.Wait(ctx, stakingOperation, coinbase.WithWaitTimeoutSeconds(60))
if err != nil {
log.Fatalf("error waiting for staking operation: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/auth/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
"Correlation-Context",
fmt.Sprintf(
"%s,%s",
fmt.Sprintf("%s=%s", "sdk_version", "0.0.10"),
fmt.Sprintf("%s=%s", "sdk_version", "0.0.11"),
fmt.Sprintf("%s=%s", "sdk_language", "go"),
),
)
Expand Down
38 changes: 24 additions & 14 deletions pkg/coinbase/staking_operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (s *StakingOperation) GetSignedVoluntaryExitMessages() ([]string, error) {
return signedVoluntaryExitMessages, nil
}

func (c *Client) Wait(ctx context.Context, stakingOperation *StakingOperation, o ...WaitOption) (*StakingOperation, error) {
func (c *Client) Wait(ctx context.Context, stakingOperation *StakingOperation, o ...WaitOption) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im okay with this change, but curious as to why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Wait interface attempts to relieve control back to user when the staking operation has terminated along with an up-to-date staking operation object. The options are b/w returning back an updated object vs doing in-place.

Since we pass the staking operation by reference it's possible to do an in-place update. Returning back an explicit staking operation object in response didn't feel the right thing to do. Made it slightly more verbose that what I had imagined it to be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like Wait() should be a method on the StakingOperation struct in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ Ideally yes - but in the Go SDK all operations are mostly performed directly on the client directly. Doing on wait on staking operation would require access to the client to make backend calls but the SDK is not setup for that (I believe that is intentional)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the "Go" way since the actions are happening on the API (ie Client) not on the structs. Structs have functions for local changes, client has remote change mutations.

options := &waitOptions{
intervalSeconds: 5,
timeoutSeconds: 3600,
Expand All @@ -206,50 +206,60 @@ func (c *Client) Wait(ctx context.Context, stakingOperation *StakingOperation, o
startTime := time.Now()

for time.Since(startTime).Seconds() < float64(options.timeoutSeconds) {
so, err := c.fetchExternalStakingOperation(ctx, stakingOperation)
if err != nil {
return stakingOperation, err
if err := c.ReloadStakingOperation(ctx, stakingOperation); err != nil {
return err
}
stakingOperation = so

if stakingOperation.isTerminalState() {
return stakingOperation, nil
return nil
}

if time.Since(startTime).Seconds() > float64(options.timeoutSeconds) {
return stakingOperation, fmt.Errorf("staking operation timed out")
return fmt.Errorf("staking operation timed out")
}

time.Sleep(time.Duration(options.intervalSeconds) * time.Second)
}

return stakingOperation, fmt.Errorf("staking operation timed out")
return fmt.Errorf("staking operation timed out")
}

// FetchExternalStakingOperation reloads a staking operation from the API associated
// with an address.
func (c *Client) fetchExternalStakingOperation(ctx context.Context, stakingOperation *StakingOperation) (*StakingOperation, error) {
// ReloadStakingOperation reloads a staking operation from the backend with the latest state.
// It ensures only newly constructed transactions are added to the staking operation and any existing transactions are untouched.
func (c *Client) ReloadStakingOperation(ctx context.Context, stakingOperation *StakingOperation) error {
so, httpResp, err := c.client.StakeAPI.GetExternalStakingOperation(
ctx,
stakingOperation.NetworkID(),
stakingOperation.AddressID(),
stakingOperation.ID(),
).Execute()
if err != nil {
return nil, errors.MapToUserFacing(err, httpResp)
return errors.MapToUserFacing(err, httpResp)
}

stakingOperation.model = so

for _, tx := range so.Transactions {
if !stakingOperation.hasTransactionByUnsignedPayload(tx.UnsignedPayload) {
newTx, err := newTransactionFromModel(&tx)
if err != nil {
return nil, err
return err
}

stakingOperation.transactions = append(stakingOperation.transactions, newTx)
}
}
return stakingOperation, nil
return nil
}

// FetchStakingOperation fetches a staking operation from the backend given a networkID, addressID, and stakingOperationID.
func (c *Client) FetchStakingOperation(ctx context.Context, networkID, addressID, stakingOperationID string) (*StakingOperation, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same name we use in other SDKs? I thought we included the word external for some reason? (since you can fetch a wallet based op that is slightly different?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In both node and ruby we seem to call it just fetch which takes an optional wallet id as input based on which GetExternalStakingOperation and GetStakingOperation is called.

Since Go SDK is purely external address based for now, I decided not to introduce the wallet id param and simply call GetExternalStakingOperation internally.

so, httpResp, err := c.client.StakeAPI.GetExternalStakingOperation(ctx, networkID, addressID, stakingOperationID).Execute()
if err != nil {
return nil, errors.MapToUserFacing(err, httpResp)
}

return newStakingOperationFromModel(so)
}

func (s *StakingOperation) hasTransactionByUnsignedPayload(unsignedPayload string) bool {
Expand Down
203 changes: 194 additions & 9 deletions pkg/coinbase/staking_operation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func (s *StakingOperationSuite) TestStakingOperation_Wait_Success() {

c := &Client{
client: &api.APIClient{
StakeAPI: mc.stakeAPI,
AssetsAPI: mc.assetsAPI,
StakeAPI: mc.stakeAPI,
},
}

Expand All @@ -47,7 +46,7 @@ func (s *StakingOperationSuite) TestStakingOperation_Wait_Success() {
s.NoError(err, "failed to sign staking operation")
signedPayload := so.Transactions()[0].SignedPayload()
s.NotEmpty(signedPayload, "signed payload should not be empty")
so, err = c.Wait(context.Background(), so)
err = c.Wait(context.Background(), so)
s.NoError(err, "staking operation wait should not error")
s.Equal("complete", so.Status(), "staking operation status should be complete")
s.Equal(1, len(so.Transactions()), "staking operation should have 1 transaction")
Expand All @@ -62,14 +61,13 @@ func (s *StakingOperationSuite) TestStakingOperation_Wait_Success_CustomOptions(

c := &Client{
client: &api.APIClient{
StakeAPI: mc.stakeAPI,
AssetsAPI: mc.assetsAPI,
StakeAPI: mc.stakeAPI,
},
}

so, err := mockStakingOperation(s.T(), "pending")
s.NoError(err, "staking operation creation should not error")
so, err = c.Wait(
err = c.Wait(
context.Background(),
so,
WithWaitIntervalSeconds(1),
Expand Down Expand Up @@ -110,14 +108,13 @@ func (s *StakingOperationSuite) TestStakingOperation_Wait_Failure() {

c := &Client{
client: &api.APIClient{
StakeAPI: mc.stakeAPI,
AssetsAPI: mc.assetsAPI,
StakeAPI: mc.stakeAPI,
},
}

so, err := mockStakingOperation(t, tt.soStatus)
s.NoError(err, "staking operation creation should not error")
_, err = c.Wait(context.Background(), so)
err = c.Wait(context.Background(), so)
s.Error(err, "staking operation wait should error")
})
}
Expand Down Expand Up @@ -167,6 +164,7 @@ func mockStakingOperation(t *testing.T, status string) (*StakingOperation, error

func mockGetExternalStakingOperation(t *testing.T, stakeAPI *mocks.StakeAPI, statusCode int, soStatus string) {
t.Helper()

stakeAPI.On("GetExternalStakingOperation", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
api.ApiGetExternalStakingOperationRequest{ApiService: stakeAPI},
).Once()
Expand Down Expand Up @@ -227,6 +225,18 @@ func mockGetExternalStakingOperation(t *testing.T, stakeAPI *mocks.StakeAPI, sta
).Once()
}

func mockGetExternalStakingOperationWithData(t *testing.T, stakingAPI *mocks.StakeAPI, statusCode int, op *api.StakingOperation) {
t.Helper()

stakingAPI.On("GetExternalStakingOperation", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
api.ApiGetExternalStakingOperationRequest{ApiService: stakingAPI},
).Once()

stakingAPI.On("GetExternalStakingOperationExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
op, &http.Response{StatusCode: statusCode}, nil,
).Once()
}

func (s *StakingOperationSuite) TestSign_AllTransactionsSigned() {
// Create mock transactions
signable1 := new(mocks.Signable)
Expand Down Expand Up @@ -333,3 +343,178 @@ func (s *StakingOperationSuite) TestSign_SignTransactionFails() {
signable1.AssertNotCalled(s.T(), "Sign", mock.Anything)
signable2.AssertCalled(s.T(), "Sign", signer)
}

func (s *StakingOperationSuite) TestReloadStakingOperation_ExistingTransactionsNotOverwritten() {
var (
networkID = "ethereum-holesky"
addressID = "0x14a34"
stakingOperationID = "staking-operation-id"
stakingOperationStatus = "pending"
existingUnsignedPayload = dummyEthereumUnsignedPayload(s.T(), 0)
newUnsignedPayload = dummyEthereumUnsignedPayload(s.T(), 1)
stakingAPIMock = mocks.NewStakeAPI(s.T())

newStakingOperation = &api.StakingOperation{
Id: stakingOperationID,
NetworkId: networkID,
AddressId: addressID,
Status: stakingOperationStatus,
Transactions: []api.Transaction{
{
NetworkId: networkID,
Status: "pending",
UnsignedPayload: existingUnsignedPayload,
},
{
NetworkId: networkID,
Status: "pending",
UnsignedPayload: newUnsignedPayload,
},
},
}
)

mockGetExternalStakingOperationWithData(s.T(), stakingAPIMock, http.StatusOK, newStakingOperation)

c := &Client{
client: &api.APIClient{
StakeAPI: stakingAPIMock,
},
}

// Create a staking operation with an existing transaction
stakingOp := &StakingOperation{
model: &client.StakingOperation{
Id: stakingOperationID,
NetworkId: networkID,
AddressId: addressID,
Status: stakingOperationStatus,
},
transactions: []*Transaction{
{
model: &client.Transaction{
UnsignedPayload: existingUnsignedPayload,
},
},
},
}

err := c.ReloadStakingOperation(context.Background(), stakingOp)
s.NoError(err, "reload staking operation should not error")

// Ensure the existing transaction is not overwritten
s.Equal(2, len(stakingOp.transactions), "staking operation should have 1 transaction")
s.Equal(existingUnsignedPayload, stakingOp.transactions[0].UnsignedPayload(), "existing transaction should not be overwritten")
s.Equal(newUnsignedPayload, stakingOp.transactions[1].UnsignedPayload(), "new transaction should be added")
}

func (s *StakingOperationSuite) TestReloadStakingOperation_ErrorFormatting() {
var (
networkID = "ethereum-holesky"
addressID = "0x14a34"
stakingOperationID = "staking-operation-id"
stakingOperationStatus = "pending"
stakingAPIMock = mocks.NewStakeAPI(s.T())
)

stakingAPIMock.On("GetExternalStakingOperation", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
api.ApiGetExternalStakingOperationRequest{ApiService: stakingAPIMock},
).Once()

stakingAPIMock.On("GetExternalStakingOperationExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
nil, nil, fmt.Errorf("backend error"),
).Once()

c := &Client{
client: &api.APIClient{
StakeAPI: stakingAPIMock,
},
}

// Create a staking operation with an existing transaction
stakingOp := &StakingOperation{
model: &client.StakingOperation{
Id: stakingOperationID,
NetworkId: networkID,
AddressId: addressID,
Status: stakingOperationStatus,
},
}

// Call ReloadStakingOperation
err := c.ReloadStakingOperation(context.Background(), stakingOp)
s.Error(err, "reload staking operation should error")
s.Contains(err.Error(), "backend error", "error message should be user-facing")
}

func (s *StakingOperationSuite) TestFetchStakingOperation_Success() {
var (
networkID = "ethereum-holesky"
addressID = "0x14a34"
stakingOperationID = "staking-operation-id"
stakingOperationStatus = "pending"
unsignedPayload = dummyEthereumUnsignedPayload(s.T(), 0)
stakingAPIMock = mocks.NewStakeAPI(s.T())

fetchedStakingOperation = &api.StakingOperation{
Id: stakingOperationID,
NetworkId: networkID,
AddressId: addressID,
Status: stakingOperationStatus,
Transactions: []api.Transaction{
{
NetworkId: networkID,
Status: "pending",
UnsignedPayload: unsignedPayload,
},
},
}
)

mockGetExternalStakingOperationWithData(s.T(), stakingAPIMock, http.StatusOK, fetchedStakingOperation)

c := &Client{
client: &api.APIClient{
StakeAPI: stakingAPIMock,
},
}

stakingOp, err := c.FetchStakingOperation(context.Background(), networkID, addressID, stakingOperationID)
s.NoError(err, "fetch staking operation should not error")

// Ensure the fetched staking operation is correct
s.Equal(stakingOperationID, stakingOp.ID(), "staking operation ID should match")
s.Equal(networkID, stakingOp.NetworkID(), "network ID should match")
s.Equal(addressID, stakingOp.AddressID(), "address ID should match")
s.Equal(stakingOperationStatus, stakingOp.Status(), "status should match")
s.Equal(1, len(stakingOp.Transactions()), "staking operation should have 1 transaction")
s.Equal(unsignedPayload, stakingOp.Transactions()[0].UnsignedPayload(), "transaction unsigned payload should match")
}

func (s *StakingOperationSuite) TestFetchStakingOperation_Error() {
var (
networkID = "ethereum-holesky"
addressID = "0x14a34"
stakingOperationID = "staking-operation-id"
stakingAPIMock = mocks.NewStakeAPI(s.T())
)

stakingAPIMock.On("GetExternalStakingOperation", mock.Anything, networkID, addressID, stakingOperationID).Return(
api.ApiGetExternalStakingOperationRequest{ApiService: stakingAPIMock},
).Once()

stakingAPIMock.On("GetExternalStakingOperationExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
nil, nil, fmt.Errorf("backend error"),
).Once()

c := &Client{
client: &api.APIClient{
StakeAPI: stakingAPIMock,
},
}

stakingOp, err := c.FetchStakingOperation(context.Background(), networkID, addressID, stakingOperationID)
s.Error(err, "fetch staking operation should error")
s.Nil(stakingOp, "staking operation should be nil on error")
s.Contains(err.Error(), "backend error", "error message should be user-facing")
}
Loading