feat: Update wait interface, Add FetchStakingOperation and ReloadStakingOperation #37
Conversation
…n id, and make reload method public to reload a staking operation from backend
✅ Heimdall Review Status
|
| @@ -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 { | |||
There was a problem hiding this comment.
Im okay with this change, but curious as to why?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
It feels like Wait() should be a method on the StakingOperation struct in this case.
There was a problem hiding this comment.
^ 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)
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| // 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) { |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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.
What changed? Why?
This PR attempts to expose some helper methods to deal with staking operations that were previously only accessible via a generic helper around waiting for staking operations to complete (Wait).
Qualified Impact