Skip to content

V1 Deprecation: Remove algod and kmd client affinity and consolidate APIs to use newest version #4641

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.

Sign up for GitHub

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

Merged
michaeldiamant merged 2 commits into from
Oct 18, 2022
Merged

V1 Deprecation: Remove algod and kmd client affinity and consolidate APIs to use newest version #4641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86fd07e
Delete kmd API affinity
algochoi Oct 12, 2022
c7f6cea
Delete affinity in algod
algochoi Oct 12, 2022
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: 0 additions & 4 deletions cmd/goal/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ import (
"github.com/spf13/cobra/doc"
"golang.org/x/crypto/ssh/terminal"

algodclient "github.com/algorand/go-algorand/daemon/algod/api/client"
kmdclient "github.com/algorand/go-algorand/daemon/kmd/client"

"github.com/algorand/go-algorand/config"
"github.com/algorand/go-algorand/daemon/algod/api/spec/common"
"github.com/algorand/go-algorand/data/bookkeeping"
Expand Down Expand Up @@ -381,7 +378,6 @@ func getGoalClient(dataDir string, clientType libgoal.ClientType) (client libgoa
if err != nil {
return
}
client.SetAPIVersionAffinity(algodclient.APIVersionV2, kmdclient.APIVersionV1)
return
}

Expand Down
74 changes: 10 additions & 64 deletions daemon/algod/api/client/restClient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,6 @@ const (
maxRawResponseBytes = 50e6
)

// APIVersion is used to define which server side API version would be used when making http requests to the server
type APIVersion string

const (
// APIVersionV1 suggests that the RestClient would use v1 calls whenever it's available for the given request.
APIVersionV1 APIVersion = "v1"
// APIVersionV2 suggests that the RestClient would use v2 calls whenever it's available for the given request.
APIVersionV2 APIVersion = "v2"
)

// rawRequestPaths is a set of paths where the body should not be urlencoded
var rawRequestPaths = map[string]bool{
"/v1/transactions": true,
Expand Down Expand Up @@ -91,27 +81,18 @@ func (e HTTPError) Error() string {

// RestClient manages the REST interface for a calling user.
type RestClient struct {
serverURL url.URL
apiToken string
versionAffinity APIVersion
serverURL url.URL
apiToken string
}

// MakeRestClient is the factory for constructing a RestClient for a given endpoint
func MakeRestClient(url url.URL, apiToken string) RestClient {
return RestClient{
serverURL: url,
apiToken: apiToken,
versionAffinity: APIVersionV1,
serverURL: url,
apiToken: apiToken,
Comment on lines 89 to +92
Copy link
Contributor

@jannotti jannotti Oct 14, 2022

Choose a reason for hiding this comment

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

Do we understand why this was using APIVersionV1? It means, for example, that the loadgenerator cmd/loadgenerator/main.go uses V1 today.

Copy link
Contributor

@michaeldiamant michaeldiamant Oct 18, 2022

Choose a reason for hiding this comment

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

I'm not sure.

  • Scanning git history, I see 1750fb5#diff-c8e8e0d825d7542580daf188e1c5f6b4f079c888d10dfb2544a8a75b6419d0b4R72. When version affinity was introduced, MakeRestClient was locked to v1.
  • My unsubstantiated guess is: Using v1 simplified other ongoing work at the time and switching to v2 got lost in the shuffle.
  • Additionally - For load generator algod API usage, I scanned endpoint implementations in v1 + v2 and spotted no meaningful differences. Of course, it'd be preferable for someone more familiar with load generator to vouch and/or run a test.

}
}

// SetAPIVersionAffinity sets the client affinity to use a specific version of the API
func (client *RestClient) SetAPIVersionAffinity(affinity APIVersion) (previousAffinity APIVersion) {
previousAffinity = client.versionAffinity
client.versionAffinity = affinity
return
}

// filterASCII filter out the non-ascii printable characters out of the given input string.
// It's used as a security qualifier before adding network provided data into an error message.
// The function allows only characters in the range of [32..126], which excludes all the
Expand Down Expand Up @@ -251,31 +232,13 @@ func (client RestClient) post(response interface{}, path string, request interfa
// the StatusResponse includes data like the consensus version and current round
// Not supported
func (client RestClient) Status() (response generatedV2.NodeStatusResponse, err error) {
switch client.versionAffinity {
case APIVersionV2:
err = client.get(&response, "/v2/status", nil)
default:
var nodeStatus v1.NodeStatus
err = client.get(&nodeStatus, "/v1/status", nil)
if err == nil {
response = fillNodeStatusResponse(nodeStatus)
}
}
err = client.get(&response, "/v2/status", nil)
return
}

// WaitForBlock returns the node status after waiting for the given round.
func (client RestClient) WaitForBlock(round basics.Round) (response generatedV2.NodeStatusResponse, err error) {
switch client.versionAffinity {
case APIVersionV2:
err = client.get(&response, fmt.Sprintf("/v2/status/wait-for-block-after/%d/", round), nil)
default:
var nodeStatus v1.NodeStatus
err = client.get(&nodeStatus, fmt.Sprintf("/v1/status/wait-for-block-after/%d/", round), nil)
if err == nil {
response = fillNodeStatusResponse(nodeStatus)
}
}
err = client.get(&response, fmt.Sprintf("/v2/status/wait-for-block-after/%d/", round), nil)
return
}

Expand All @@ -302,17 +265,7 @@ func fillNodeStatusResponse(nodeStatus v1.NodeStatus) generatedV2.NodeStatusResp
// blocks on the node end
// Not supported
func (client RestClient) StatusAfterBlock(blockNum uint64) (response generatedV2.NodeStatusResponse, err error) {
switch client.versionAffinity {
case APIVersionV2:
err = client.get(&response, fmt.Sprintf("/v2/status/wait-for-block-after/%d", blockNum), nil)
default:
var nodeStatus v1.NodeStatus
err = client.get(&nodeStatus, fmt.Sprintf("/v1/status/wait-for-block-after/%d", blockNum), nil)
if err == nil {
response = fillNodeStatusResponse(nodeStatus)
}
}

err = client.get(&response, fmt.Sprintf("/v2/status/wait-for-block-after/%d", blockNum), nil)
return
}

Expand Down Expand Up @@ -546,16 +499,9 @@ func (client RestClient) Block(round uint64) (response v1.Block, err error) {

// RawBlock gets the encoded, raw msgpack block for the given round
func (client RestClient) RawBlock(round uint64) (response []byte, err error) {
switch client.versionAffinity {
case APIVersionV2:
var blob Blob
err = client.getRaw(&blob, fmt.Sprintf("/v2/blocks/%d", round), rawFormat{Format: "msgpack"})
response = blob
default:
var raw v1.RawBlock
err = client.getRaw(&raw, fmt.Sprintf("/v1/block/%d", round), rawblockParams{1})
response = raw
}
var blob Blob
err = client.getRaw(&blob, fmt.Sprintf("/v2/blocks/%d", round), rawFormat{Format: "msgpack"})
response = blob
return
}

Expand Down
8 changes: 0 additions & 8 deletions daemon/kmd/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ const (
timeoutSecs = 120
)

// APIVersion is used to define which server side API version would be used when making http requests to the server
type APIVersion string

const (
// APIVersionV1 suggests that the RestClient would use v1 calls whenever it's available for the given request.
APIVersionV1 APIVersion = "v1"
)

// KMDClient is the client used to interact with the kmd API over its socket
type KMDClient struct {
httpClient http.Client
Expand Down
22 changes: 5 additions & 17 deletions libgoal/libgoal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,11 @@ const DefaultKMDDataDir = nodecontrol.DefaultKMDDataDir

// Client represents the entry point for all libgoal functions
type Client struct {
nc nodecontrol.NodeController
kmdStartArgs nodecontrol.KMDStartArgs
dataDir string
cacheDir string
consensus config.ConsensusProtocols
algodVersionAffinity algodclient.APIVersion
kmdVersionAffinity kmdclient.APIVersion
nc nodecontrol.NodeController
kmdStartArgs nodecontrol.KMDStartArgs
dataDir string
cacheDir string
consensus config.ConsensusProtocols

suggestedParamsCache v1.TransactionParams
suggestedParamsExpire time.Time
Expand Down Expand Up @@ -148,8 +146,6 @@ func (c *Client) init(config ClientConfig, clientType ClientType) error {
}
c.dataDir = dataDir
c.cacheDir = config.CacheDir
c.algodVersionAffinity = algodclient.APIVersionV1
c.kmdVersionAffinity = kmdclient.APIVersionV1

// Get node controller
nc, err := getNodeController(config.BinDir, config.AlgodDataDir)
Expand Down Expand Up @@ -204,7 +200,6 @@ func (c *Client) ensureAlgodClient() (*algodclient.RestClient, error) {
if err != nil {
return nil, err
}
algod.SetAPIVersionAffinity(c.algodVersionAffinity)
return &algod, err
}

Expand Down Expand Up @@ -1053,20 +1048,13 @@ func (c *Client) ConsensusParams(round uint64) (consensus config.ConsensusParams
return params, nil
}

// SetAPIVersionAffinity sets the desired client API version affinity of the algod and kmd clients.
func (c *Client) SetAPIVersionAffinity(algodVersionAffinity algodclient.APIVersion, kmdVersionAffinity kmdclient.APIVersion) {
c.algodVersionAffinity = algodVersionAffinity
c.kmdVersionAffinity = kmdVersionAffinity
}

// AbortCatchup aborts the currently running catchup
func (c *Client) AbortCatchup() error {
algod, err := c.ensureAlgodClient()
if err != nil {
return err
}
// we need to ensure we're using the v2 status so that we would get the catchpoint information.
algod.SetAPIVersionAffinity(algodclient.APIVersionV2)
resp, err := algod.Status()
if err != nil {
return err
Expand Down
2 changes: 0 additions & 2 deletions test/e2e-go/features/catchup/catchpointCatchup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ func TestBasicCatchpointCatchup(t *testing.T) {
targetCatchpointRound := (basics.Round(expectedBlocksToDownload+minRound)/catchpointInterval + 1) * catchpointInterval
targetRound := uint64(targetCatchpointRound) + 1
primaryNodeRestClient := fixture.GetAlgodClientForController(primaryNode)
primaryNodeRestClient.SetAPIVersionAffinity(algodclient.APIVersionV2)
log.Infof("Building ledger history..")
for {
err = fixture.ClientWaitForRound(primaryNodeRestClient, currentRound, 45*time.Second)
Expand Down Expand Up @@ -378,7 +377,6 @@ func TestCatchpointLabelGeneration(t *testing.T) {
currentRound := uint64(1)
targetRound := uint64(21)
primaryNodeRestClient := fixture.GetAlgodClientForController(primaryNode)
primaryNodeRestClient.SetAPIVersionAffinity(algodclient.APIVersionV2)
log.Infof("Building ledger history..")
for {
err = fixture.ClientWaitForRound(primaryNodeRestClient, currentRound, 45*time.Second)
Expand Down
6 changes: 0 additions & 6 deletions test/e2e-go/restAPI/restClient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ import (
"github.com/algorand/go-algorand/config"
"github.com/algorand/go-algorand/crypto"
"github.com/algorand/go-algorand/crypto/merklesignature"
algodclient "github.com/algorand/go-algorand/daemon/algod/api/client"
v1 "github.com/algorand/go-algorand/daemon/algod/api/spec/v1"
kmdclient "github.com/algorand/go-algorand/daemon/kmd/client"
"github.com/algorand/go-algorand/data/account"
"github.com/algorand/go-algorand/data/basics"
"github.com/algorand/go-algorand/data/transactions"
Expand Down Expand Up @@ -201,7 +199,6 @@ func TestClientCanGetStatus(t *testing.T) {
statusResponse, err := testClient.Status()
a.NoError(err)
a.NotEmpty(statusResponse)
testClient.SetAPIVersionAffinity(algodclient.APIVersionV2, kmdclient.APIVersionV1)
statusResponse2, err := testClient.Status()
a.NoError(err)
a.NotEmpty(statusResponse2)
Expand All @@ -218,7 +215,6 @@ func TestClientCanGetStatusAfterBlock(t *testing.T) {
statusResponse, err := testClient.WaitForRound(1)
a.NoError(err)
a.NotEmpty(statusResponse)
testClient.SetAPIVersionAffinity(algodclient.APIVersionV2, kmdclient.APIVersionV1)
statusResponse, err = testClient.WaitForRound(statusResponse.LastRound + 1)
a.NoError(err)
a.NotEmpty(statusResponse)
Expand Down Expand Up @@ -955,8 +951,6 @@ func TestPendingTransactionInfoInnerTxnAssetCreate(t *testing.T) {

testClient.WaitForRound(1)

testClient.SetAPIVersionAffinity(algodclient.APIVersionV2, kmdclient.APIVersionV1)

wh, err := testClient.GetUnencryptedWalletHandle()
a.NoError(err)
addresses, err := testClient.ListAddresses(wh)
Expand Down