Skip to content
New issue

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

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

Already on GitHub? # to your account

Upgrade flow go sdk #296

Merged
merged 9 commits into from
Jul 1, 2022
Merged

Conversation

elizabethengelman
Copy link
Contributor

@elizabethengelman elizabethengelman commented Jun 9, 2022

This pull request updates flow-go-sdk to v0.26.0, and cadence to v0.24.0. Both of these dependencies include breaking changes included in the Secure Cadence release, so there we some additional updates to the source code to conform with the new dependencies. This should handle issue #295.

@@ -359,11 +359,14 @@ func (s *ServiceImpl) createAccount(ctx context.Context) (*Account, string, erro
publicKeys = append(publicKeys, &clonedAccountKey)
}

flowTx := flow_templates.CreateAccount(
flowTx, err := flow_templates.CreateAccount(
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 CreateAccount method now returns the created tx and an error.

@@ -4,7 +4,7 @@ package errors
import (
"net"

"github.com/onflow/flow-go-sdk/client"
"github.com/onflow/flow-go-sdk/access/grpc"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing the deprecated client pkg with access pkg.

@@ -316,7 +316,7 @@ func (s *ServiceImpl) sendTransaction(ctx context.Context, tx *Transaction) erro
// Check if transaction has been sent already.
_, err = s.fc.GetTransaction(ctx, flowTx.ID())
if err != nil {
rpcErr, ok := err.(client.RPCError)
rpcErr, ok := err.(grpc.RPCError)
Copy link
Contributor Author

@elizabethengelman elizabethengelman Jun 9, 2022

Choose a reason for hiding this comment

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

Again, replacing client pkg with access since client has been deprecated.

@@ -22,7 +22,7 @@ func ArgAsCadence(a Argument) (cadence.Value, error) {
}

// Use cadence's own encoding library
c, err = c_json.Decode(j)
c, err = c_json.Decode(nil, j)
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 cadence json Decode method now requires that a memory gauge be passed in as an arg.

I could use some guidance about what this value should be. In some of the tests in the cadence repo, nil was passed in, but I'm not sure if that's what we want here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

MemoryGauge has a single method MeterMemory that apparently can be used to pass information on memory usage back to the caller. As UseMemory manages just fine with nil and memory usage information is not used, it can be left nil.

@@ -9,7 +9,7 @@ import (
"github.com/flow-hydraulics/flow-wallet-api/configs"
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 changes in this file are similar to previous changes:

  • using the access pkg instead of the deprecated client package
  • CreateAccount now returns the tx and an error

)

type MockFlowClient struct {
getTransactionResultCallCount uint
}

func (c *MockFlowClient) ExecuteScriptAtLatestBlock(ctx context.Context, script []byte, arguments []cadence.Value, opts ...grpc.CallOption) (cadence.Value, error) {
func (c *MockFlowClient) ExecuteScriptAtLatestBlock(ctx context.Context, script []byte, arguments []cadence.Value) (cadence.Value, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is the same as the previous file - the new access pkg client interface has changed.

@@ -213,6 +222,10 @@ func (s *AWSSigner) Sign(message []byte) ([]byte, error) {
return sig, nil
}

func (s *AWSSigner) PublicKey() crypto.PublicKey {
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 crypto.Signer interface now requires a PublicKey method: https://github.com/onflow/flow-go-sdk/blob/v0.26.0/crypto/crypto.go#L77

@@ -51,5 +51,5 @@ func Signer(ctx context.Context, key keys.Private) (crypto.Signer, error) {
if err != nil {
return crypto.InMemorySigner{}, err
}
return crypto.NewInMemorySigner(p, key.HashAlgo), nil
return crypto.NewInMemorySigner(p, key.HashAlgo)
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 crypto.NewInMemorySigner method now returns an error along with the new signer: https://github.com/onflow/flow-go-sdk/blob/v0.26.0/crypto/crypto.go#L99

StartHeight: start,
EndHeight: end,
})
r, err := l.fc.GetEventsForHeightRange(ctx, t, start, end)
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 flow client (fc) passed into the listener is now an instance of the access.Client interface, which has a different method signature for GetEventsForHeightRange than it previously did.

@@ -23,7 +23,7 @@ import (
"github.com/flow-hydraulics/flow-wallet-api/transactions"
"github.com/gomodule/redigo/redis"
"github.com/gorilla/mux"
"github.com/onflow/flow-go-sdk/client"
access "github.com/onflow/flow-go-sdk/access/grpc"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as is mentioned above, replacing the deprecated client pkg with access.

@elizabethengelman elizabethengelman marked this pull request as ready for review June 9, 2022 20:02
@nanuuki nanuuki requested a review from aki-eiger June 30, 2022 11:32
Copy link
Collaborator

@aki-eiger aki-eiger left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the helpful comments for each change @elizabethengelman

@@ -22,7 +22,7 @@ func ArgAsCadence(a Argument) (cadence.Value, error) {
}

// Use cadence's own encoding library
c, err = c_json.Decode(j)
c, err = c_json.Decode(nil, j)
Copy link
Collaborator

Choose a reason for hiding this comment

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

MemoryGauge has a single method MeterMemory that apparently can be used to pass information on memory usage back to the caller. As UseMemory manages just fine with nil and memory usage information is not used, it can be left nil.

@aki-eiger aki-eiger merged commit 9a47f3b into flow-hydraulics:main Jul 1, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants