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
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
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 @@ -238,16 +237,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
64 changes: 37 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,34 @@ 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 {
return err
}

switch status {
case choices.Accepted:
return nil
case choices.Rejected:
return ErrRejected
}

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

const defaultPollingInterval = 50 * time.Millisecond
const DefaultPollingInterval = 50 * time.Millisecond

// Client defines the xsvm API client.
type Client interface {
Expand Down Expand Up @@ -245,20 +245,23 @@ 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 {
// The nonce increasing indicates the acceptance of a transaction
// issued with the specified nonce.
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, api.DefaultPollingInterval); err != nil {
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, api.DefaultPollingInterval); 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, api.DefaultPollingInterval); err != nil {
return nil, err
}

Expand Down
60 changes: 29 additions & 31 deletions vms/platformvm/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,6 @@ type Client interface {
GetTx(ctx context.Context, txID ids.ID, options ...rpc.Option) ([]byte, error)
// GetTxStatus returns the status of the transaction corresponding to [txID]
GetTxStatus(ctx context.Context, txID ids.ID, options ...rpc.Option) (*GetTxStatusResponse, error)
// AwaitTxDecided polls [GetTxStatus] until a status is returned that
// implies the tx may be decided.
// TODO: Move this function off of the Client interface into a utility
// function.
AwaitTxDecided(
ctx context.Context,
txID ids.ID,
freq time.Duration,
options ...rpc.Option,
) (*GetTxStatusResponse, error)
// GetStake returns the amount of nAVAX that [addrs] have cumulatively
// staked on the Primary Network.
//
Expand Down Expand Up @@ -409,27 +399,6 @@ func (c *client) GetTxStatus(ctx context.Context, txID ids.ID, options ...rpc.Op
return res, err
}

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

for {
res, err := c.GetTxStatus(ctx, txID, options...)
if err == nil {
switch res.Status {
case status.Committed, status.Aborted, status.Dropped:
return res, nil
}
}

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

func (c *client) GetStake(
ctx context.Context,
addrs []ids.ShortID,
Expand Down Expand Up @@ -545,3 +514,32 @@ func (c *client) GetBlockByHeight(ctx context.Context, height uint64, options ..
}
return formatting.Decode(res.Encoding, res.Block)
}

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 {
res, err := c.GetTxStatus(ctx, txID, options...)
if err != nil {
return err
}

switch res.Status {
case status.Committed, status.Aborted:
return nil
}

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