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

Standardize wallet tx acceptance polling #3110

Merged
merged 10 commits into from
Jun 14, 2024
28 changes: 2 additions & 26 deletions tests/antithesis/avalanchego/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/ava-labs/avalanchego/database"
"github.com/ava-labs/avalanchego/genesis"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow/choices"
"github.com/ava-labs/avalanchego/tests/antithesis"
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
Expand All @@ -27,7 +26,6 @@ import (
"github.com/ava-labs/avalanchego/vms/components/avax"
"github.com/ava-labs/avalanchego/vms/components/verify"
"github.com/ava-labs/avalanchego/vms/platformvm"
"github.com/ava-labs/avalanchego/vms/platformvm/status"
"github.com/ava-labs/avalanchego/vms/propertyfx"
"github.com/ava-labs/avalanchego/vms/secp256k1fx"
"github.com/ava-labs/avalanchego/wallet/subnet/primary"
Expand Down Expand Up @@ -581,8 +579,7 @@ func (w *workload) confirmXChainTx(ctx context.Context, tx *xtxs.Tx) {
txID := tx.ID()
for _, uri := range w.uris {
client := avm.NewClient(uri, "X")
status, err := client.ConfirmTx(ctx, txID, 100*time.Millisecond)
if err != nil {
if err := avm.AwaitTxAccepted(client, ctx, txID, 100*time.Millisecond); err != nil {
log.Printf("failed to confirm X-chain transaction %s on %s: %s", txID, uri, err)
assert.Unreachable("failed to determine the status of an X-chain transaction", map[string]any{
"worker": w.id,
Expand All @@ -592,16 +589,6 @@ func (w *workload) confirmXChainTx(ctx context.Context, tx *xtxs.Tx) {
})
return
}
if status != choices.Accepted {
log.Printf("failed to confirm X-chain transaction %s on %s: status == %s", txID, uri, status)
assert.Unreachable("failed to confirm an X-chain transaction", map[string]any{
"worker": w.id,
"txID": txID,
"uri": uri,
"status": status,
})
return
}
log.Printf("confirmed X-chain transaction %s on %s", txID, uri)
}
log.Printf("confirmed X-chain transaction %s on all nodes", txID)
Expand All @@ -611,8 +598,7 @@ func (w *workload) confirmPChainTx(ctx context.Context, tx *ptxs.Tx) {
txID := tx.ID()
for _, uri := range w.uris {
client := platformvm.NewClient(uri)
s, err := client.AwaitTxDecided(ctx, txID, 100*time.Millisecond)
if err != nil {
if err := platformvm.AwaitTxAccepted(client, ctx, txID, 100*time.Millisecond); err != nil {
log.Printf("failed to determine the status of a P-chain transaction %s on %s: %s", txID, uri, err)
assert.Unreachable("failed to determine the status of a P-chain transaction", map[string]any{
"worker": w.id,
Expand All @@ -622,16 +608,6 @@ func (w *workload) confirmPChainTx(ctx context.Context, tx *ptxs.Tx) {
})
return
}
if s.Status != status.Committed {
log.Printf("failed to confirm P-chain transaction %s on %s: status == %s", txID, uri, s.Status)
assert.Unreachable("failed to confirm a P-chain transaction", map[string]any{
"worker": w.id,
"txID": txID,
"uri": uri,
"status": s.Status,
})
return
}
log.Printf("confirmed P-chain transaction %s on %s", txID, uri)
}
log.Printf("confirmed P-chain transaction %s on all nodes", txID)
Expand Down
7 changes: 5 additions & 2 deletions tests/antithesis/xsvm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import (
"github.com/ava-labs/avalanchego/vms/example/xsvm/cmd/issue/transfer"
)

const NumKeys = 5
const (
NumKeys = 5
PollingInterval = 50 * time.Millisecond
)

func main() {
c, err := antithesis.NewConfig(os.Args)
Expand Down Expand Up @@ -171,7 +174,7 @@ func (w *workload) run(ctx context.Context) {
func (w *workload) confirmTransferTx(ctx context.Context, tx *status.TxIssuance) {
for _, uri := range w.uris {
client := api.NewClient(uri, w.chainID.String())
if err := api.WaitForAcceptance(ctx, client, w.key.Address(), tx.Nonce); err != nil {
if err := api.AwaitTxAccepted(ctx, client, w.key.Address(), tx.Nonce, PollingInterval); err != nil {
log.Printf("worker %d failed to confirm transaction %s on %s: %s", w.id, tx.TxID, uri, err)
assert.Unreachable("failed to confirm transaction", map[string]any{
"worker": w.id,
Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/c/interchain_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
ginkgo.By("defining common configuration")
xBuilder := xWallet.Builder()
xContext := xBuilder.Context()
cContext := cWallet.Builder().Context()
avaxAssetID := xContext.AVAXAssetID
// Use the same owner for import funds to X-Chain and P-Chain
recipientOwner := secp256k1fx.OutputOwners{
Expand Down Expand Up @@ -119,7 +120,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {

ginkgo.By("importing AVAX from the C-Chain to the X-Chain", func() {
_, err := xWallet.IssueImportTx(
cWallet.BlockchainID(),
cContext.BlockchainID,
&recipientOwner,
e2e.WithDefaultContext(),
)
Expand All @@ -146,7 +147,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {

ginkgo.By("importing AVAX from the C-Chain to the P-Chain", func() {
_, err = pWallet.IssueImportTx(
cWallet.BlockchainID(),
cContext.BlockchainID,
&recipientOwner,
e2e.WithDefaultContext(),
)
Expand Down
4 changes: 3 additions & 1 deletion tests/e2e/p/interchain_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ var _ = e2e.DescribePChain("[Interchain Workflow]", ginkgo.Label(e2e.UsesCChainL
xContext := xBuilder.Context()
pBuilder := pWallet.Builder()
pContext := pBuilder.Context()
cBuilder := cWallet.Builder()
cContext := cBuilder.Context()

ginkgo.By("defining common configuration")
recipientEthAddress := evm.GetEthAddress(recipientKey)
Expand Down Expand Up @@ -186,7 +188,7 @@ var _ = e2e.DescribePChain("[Interchain Workflow]", ginkgo.Label(e2e.UsesCChainL

ginkgo.By("exporting AVAX from the P-Chain to the C-Chain", func() {
_, err := pWallet.IssueExportTx(
cWallet.BlockchainID(),
cContext.BlockchainID,
exportOutputs,
e2e.WithDefaultContext(),
)
Expand Down
6 changes: 5 additions & 1 deletion tests/e2e/vms/xsvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package vms

import (
"fmt"
"time"

"github.com/stretchr/testify/require"

Expand All @@ -22,6 +23,8 @@ import (
ginkgo "github.com/onsi/ginkgo/v2"
)

const pollingInterval = 50 * time.Millisecond

var (
subnetAName = "xsvm-a"
subnetBName = "xsvm-b"
Expand Down Expand Up @@ -85,11 +88,12 @@ var _ = ginkgo.Describe("[XSVM]", func() {

ginkgo.By("checking that the export transaction has been accepted on all nodes")
for _, node := range network.Nodes[1:] {
require.NoError(api.WaitForAcceptance(
require.NoError(api.AwaitTxAccepted(
e2e.DefaultContext(),
api.NewClient(node.URI, sourceChain.ChainID.String()),
sourceChain.PreFundedKey.Address(),
exportTxStatus.Nonce,
pollingInterval,
))
}

Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/x/interchain_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var _ = e2e.DescribeXChain("[Interchain Workflow]", ginkgo.Label(e2e.UsesCChainL
recipientEthAddress := evm.GetEthAddress(recipientKey)
xBuilder := xWallet.Builder()
xContext := xBuilder.Context()
cContext := cWallet.Builder().Context()
avaxAssetID := xContext.AVAXAssetID
// Use the same owner for sending to X-Chain and importing funds to P-Chain
recipientOwner := secp256k1fx.OutputOwners{
Expand Down Expand Up @@ -96,7 +97,7 @@ var _ = e2e.DescribeXChain("[Interchain Workflow]", ginkgo.Label(e2e.UsesCChainL

ginkgo.By("exporting AVAX from the X-Chain to the C-Chain", func() {
_, err := xWallet.IssueExportTx(
cWallet.BlockchainID(),
cContext.BlockchainID,
exportOutputs,
e2e.WithDefaultContext(),
)
Expand Down
9 changes: 2 additions & 7 deletions tests/e2e/x/transfer/virtuous.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"github.com/ava-labs/avalanchego/chains"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow/choices"
"github.com/ava-labs/avalanchego/tests"
"github.com/ava-labs/avalanchego/tests/fixture/e2e"
"github.com/ava-labs/avalanchego/utils/set"
Expand Down Expand Up @@ -236,16 +235,12 @@ RECEIVER NEW BALANCE (AFTER) : %21d AVAX
txID := tx.ID()
for _, u := range rpcEps {
xc := avm.NewClient(u, "X")
status, err := xc.ConfirmTx(e2e.DefaultContext(), txID, 2*time.Second)
require.NoError(err)
require.Equal(choices.Accepted, status)
require.NoError(avm.AwaitTxAccepted(xc, e2e.DefaultContext(), txID, 2*time.Second))
}

for _, u := range rpcEps {
xc := avm.NewClient(u, "X")
status, err := xc.ConfirmTx(e2e.DefaultContext(), txID, 2*time.Second)
require.NoError(err)
require.Equal(choices.Accepted, status)
require.NoError(avm.AwaitTxAccepted(xc, e2e.DefaultContext(), txID, 2*time.Second))

mm, err := tests.GetNodeMetrics(e2e.DefaultContext(), u)
require.NoError(err)
Expand Down
62 changes: 35 additions & 27 deletions vms/avm/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package avm

import (
"context"
"errors"
"fmt"
"time"

Expand All @@ -19,7 +20,11 @@ import (
"github.com/ava-labs/avalanchego/utils/rpc"
)

var _ Client = (*client)(nil)
var (
_ Client = (*client)(nil)

ErrRejected = errors.New("rejected")
)

// Client for interacting with an AVM (X-Chain) instance
type Client interface {
Expand All @@ -35,12 +40,6 @@ type Client interface {
// Deprecated: GetTxStatus only returns Accepted or Unknown, GetTx should be
// used instead to determine if the tx was accepted.
GetTxStatus(ctx context.Context, txID ids.ID, options ...rpc.Option) (choices.Status, error)
// ConfirmTx attempts to confirm [txID] by repeatedly checking its status.
// Note: ConfirmTx will block until either the context is done or the client
// returns a decided status.
// TODO: Move this function off of the Client interface into a utility
// function.
ConfirmTx(ctx context.Context, txID ids.ID, freq time.Duration, options ...rpc.Option) (choices.Status, error)
// GetTx returns the byte representation of [txID]
GetTx(ctx context.Context, txID ids.ID, options ...rpc.Option) ([]byte, error)
// GetUTXOs returns the byte representation of the UTXOs controlled by [addrs]
Expand Down Expand Up @@ -285,26 +284,6 @@ func (c *client) GetTxStatus(ctx context.Context, txID ids.ID, options ...rpc.Op
return res.Status, err
}

func (c *client) ConfirmTx(ctx context.Context, txID ids.ID, freq time.Duration, options ...rpc.Option) (choices.Status, error) {
ticker := time.NewTicker(freq)
defer ticker.Stop()

for {
status, err := c.GetTxStatus(ctx, txID, options...)
if err == nil {
if status.Decided() {
return status, nil
}
}

select {
case <-ticker.C:
case <-ctx.Done():
return status, ctx.Err()
}
}
}

func (c *client) GetTx(ctx context.Context, txID ids.ID, options ...rpc.Option) ([]byte, error) {
res := &api.FormattedTx{}
err := c.requester.SendRequest(ctx, "avm.getTx", &api.GetTxArgs{
Expand Down Expand Up @@ -766,3 +745,32 @@ func (c *client) Export(
}, res, options...)
return res.TxID, err
}

func AwaitTxAccepted(
c Client,
ctx context.Context,
txID ids.ID,
freq time.Duration,
options ...rpc.Option,
) error {
ticker := time.NewTicker(freq)
defer ticker.Stop()

for {
status, err := c.GetTxStatus(ctx, txID, options...)
StephenButtolph marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
switch status {
case choices.Accepted:
return nil
case choices.Rejected:
return ErrRejected
}
}

select {
case <-ticker.C:
case <-ctx.Done():
return ctx.Err()
}
}
}
13 changes: 5 additions & 8 deletions vms/example/xsvm/api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (
"github.com/ava-labs/avalanchego/vms/platformvm/warp"
)

const defaultPollingInterval = 50 * time.Millisecond

// Client defines the xsvm API client.
type Client interface {
Network(
Expand Down Expand Up @@ -245,21 +243,20 @@ func (c *client) Message(
return resp.Message, resp.Signature, resp.Message.Initialize()
}

func WaitForAcceptance(
func AwaitTxAccepted(
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 a bit uncommon go function signature. I would expect it to look like

func (c Client) AwaitTxAccepted(ctx, address, nonce, freq, options)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Client is an interface

ctx context.Context,
c Client,
address ids.ShortID,
nonce uint64,
freq time.Duration,
options ...rpc.Option,
) error {
ticker := time.NewTicker(defaultPollingInterval)
ticker := time.NewTicker(freq)
defer ticker.Stop()

for {
currentNonce, err := c.Nonce(ctx, address, options...)
if err != nil {
return err
}
if currentNonce > nonce {
if err == nil && currentNonce > nonce {
// The nonce increasing indicates the acceptance of a transaction
// issued with the specified nonce.
return nil
Expand Down
2 changes: 1 addition & 1 deletion vms/example/xsvm/cmd/issue/export/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func Export(ctx context.Context, config *Config) (*status.TxIssuance, error) {
return nil, err
}

if err := api.WaitForAcceptance(ctx, client, address, nonce); err != nil {
if err := api.AwaitTxAccepted(ctx, client, address, nonce, 50*time.Millisecond); err != nil {
StephenButtolph marked this conversation as resolved.
Show resolved Hide resolved
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion vms/example/xsvm/cmd/issue/importtx/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func Import(ctx context.Context, config *Config) (*status.TxIssuance, error) {
return nil, err
}

if err := api.WaitForAcceptance(ctx, client, address, nonce); err != nil {
if err := api.AwaitTxAccepted(ctx, client, address, nonce, 50*time.Millisecond); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion vms/example/xsvm/cmd/issue/transfer/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func Transfer(ctx context.Context, config *Config) (*status.TxIssuance, error) {
return nil, err
}

if err := api.WaitForAcceptance(ctx, client, address, nonce); err != nil {
if err := api.AwaitTxAccepted(ctx, client, address, nonce, 50*time.Millisecond); err != nil {
return nil, err
}

Expand Down
Loading
Loading