Skip to content

Commit

Permalink
feat: nonce handling with signer (#3196)
Browse files Browse the repository at this point in the history
Closes: #1910

This covers most cases by serializing the actual broadcasts to the
consensus node and enabling resubmissions in the case that there is a
sequence mismatch.

This covers most fail cases with the possible exception of proposal
nodes receiving the transactions in the reverse order to the initial
nodes that the user broadcasted to

There are also some interesting side affects that need to be handled
when an existing accepted transaction is later kicked out of the mempool
via CheckTx but overall I think this is a huge improvement for the UX of
users

(cherry picked from commit deefb54)

# Conflicts:
#	Makefile
#	app/errors/insufficient_gas_price_test.go
#	app/errors/nonce_mismatch_test.go
#	app/test/big_blob_test.go
#	app/test/priority_test.go
#	go.work.sum
#	pkg/user/signer.go
#	pkg/user/signer_test.go
#	test/util/blobfactory/payforblob_factory.go
#	test/util/blobfactory/test_util.go
#	test/util/direct_tx_gen.go
#	x/blob/types/blob_tx_test.go
  • Loading branch information
cmwaters authored and mergify[bot] committed Mar 26, 2024
1 parent fb20954 commit 1930c89
Show file tree
Hide file tree
Showing 14 changed files with 1,912 additions and 66 deletions.
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ test-short:
## test-race: Run unit tests in race mode.
test-race:
@echo "--> Running tests in race mode"
<<<<<<< HEAD
@go test ./... -v -race -skip "TestPrepareProposalConsistency|TestIntegrationTestSuite|TestQGBRPCQueries|TestSquareSizeIntegrationTest|TestStandardSDKIntegrationTestSuite|TestTxsimCommandFlags|TestTxsimCommandEnvVar|TestMintIntegrationTestSuite|TestQGBCLI|TestUpgrade|TestMaliciousTestNode|TestMaxTotalBlobSizeSuite|TestQGBIntegrationSuite|TestSignerTestSuite|TestPriorityTestSuite|TestTimeInPrepareProposalContext"
=======
@go test ./... -v -race -skip "TestPrepareProposalConsistency|TestIntegrationTestSuite|TestBlobstreamRPCQueries|TestSquareSizeIntegrationTest|TestStandardSDKIntegrationTestSuite|TestTxsimCommandFlags|TestTxsimCommandEnvVar|TestMintIntegrationTestSuite|TestBlobstreamCLI|TestUpgrade|TestMaliciousTestNode|TestBigBlobSuite|TestQGBIntegrationSuite|TestSignerTestSuite|TestPriorityTestSuite|TestTimeInPrepareProposalContext|TestBlobstream|TestCLITestSuite|TestLegacyUpgrade|TestSignerTwins|TestConcurrentTxSubmission"
>>>>>>> deefb542 (feat: nonce handling with signer (#3196))
.PHONY: test-race

## test-bench: Run unit tests in bench mode.
Expand Down
4 changes: 4 additions & 0 deletions app/errors/insufficient_gas_price_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ func TestInsufficientMinGasPriceIntegration(t *testing.T) {
pfb, err := blob.NewMsgPayForBlobs(address.String(), b)
require.NoError(t, err, address)

<<<<<<< HEAD
tx, err := signer.BuildSignedTx(builder, pfb)
=======
sdkTx, err := signer.CreateTx([]sdk.Msg{msg}, user.SetGasLimit(gasLimit), user.SetFeeAmount(fee))
>>>>>>> deefb542 (feat: nonce handling with signer (#3196))
require.NoError(t, err)

decorator := ante.NewDeductFeeDecorator(testApp.AccountKeeper, testApp.BankKeeper, testApp.FeeGrantKeeper, nil)
Expand Down
20 changes: 18 additions & 2 deletions app/errors/nonce_mismatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"strconv"
"strings"

sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Expand All @@ -13,16 +14,31 @@ func IsNonceMismatch(err error) bool {
return errors.Is(err, sdkerrors.ErrWrongSequence)
}

// IsNonceMismatch checks if the error code matches the sequence mismatch.
func IsNonceMismatchCode(code uint32) bool {
return code == sdkerrors.ErrWrongSequence.ABCICode()
}

// ParseNonceMismatch extracts the expected sequence number from the
// ErrWrongSequence error.
func ParseNonceMismatch(err error) (uint64, error) {
if !IsNonceMismatch(err) {
return 0, errors.New("error is not a sequence mismatch")
}

numbers := regexpInt.FindAllString(err.Error(), -1)
return ParseExpectedSequence(err.Error())
}

// ParseExpectedSequence extracts the expected sequence number from the
// ErrWrongSequence error.
func ParseExpectedSequence(str string) (uint64, error) {
if !strings.HasPrefix(str, "account sequence mismatch") {
return 0, fmt.Errorf("unexpected wrong sequence error: %s", str)
}

numbers := regexpInt.FindAllString(str, -1)
if len(numbers) != 2 {
return 0, fmt.Errorf("unexpected wrong sequence error: %w", err)
return 0, fmt.Errorf("expected two numbers in string, got %d", len(numbers))
}

// the first number is the expected sequence number
Expand Down
4 changes: 4 additions & 0 deletions app/errors/nonce_mismatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ func TestNonceMismatchIntegration(t *testing.T) {
pfb, err := blob.NewMsgPayForBlobs(address.String(), b)
require.NoError(t, err, address)

<<<<<<< HEAD
tx, err := signer.BuildSignedTx(builder, pfb)
=======
sdkTx, err := signer.CreateTx([]sdk.Msg{msg})
>>>>>>> deefb542 (feat: nonce handling with signer (#3196))
require.NoError(t, err)

decorator := ante.NewSigVerificationDecorator(testApp.AccountKeeper, encCfg.TxConfig.SignModeHandler())
Expand Down
89 changes: 89 additions & 0 deletions app/test/big_blob_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package app_test

import (
"context"
"testing"
"time"

"github.com/celestiaorg/celestia-app/app"
"github.com/celestiaorg/celestia-app/app/encoding"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
"github.com/celestiaorg/celestia-app/pkg/user"
"github.com/celestiaorg/celestia-app/test/util/testfactory"
"github.com/celestiaorg/celestia-app/test/util/testnode"
blobtypes "github.com/celestiaorg/celestia-app/x/blob/types"
"github.com/celestiaorg/go-square/blob"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
)

func TestBigBlobSuite(t *testing.T) {
if testing.Short() {
t.Skip("skipping big blob suite in short mode.")
}
suite.Run(t, &BigBlobSuite{})
}

type BigBlobSuite struct {
suite.Suite

ecfg encoding.Config
accounts []string
cctx testnode.Context
}

func (s *BigBlobSuite) SetupSuite() {
t := s.T()

s.accounts = testfactory.GenerateAccounts(1)

tmConfig := testnode.DefaultTendermintConfig()
tmConfig.Mempool.MaxTxBytes = 10 * mebibyte

cParams := testnode.DefaultConsensusParams()
cParams.Block.MaxBytes = 10 * mebibyte

cfg := testnode.DefaultConfig().
WithFundedAccounts(s.accounts...).
WithTendermintConfig(tmConfig).
WithConsensusParams(cParams)

cctx, _, _ := testnode.NewNetwork(t, cfg)
s.cctx = cctx
s.ecfg = encoding.MakeConfig(app.ModuleEncodingRegisters...)

require.NoError(t, cctx.WaitForNextBlock())
}

// TestErrBlobsTooLarge verifies that submitting a 2 MiB blob hits ErrBlobsTooLarge.
func (s *BigBlobSuite) TestErrBlobsTooLarge() {
t := s.T()

type testCase struct {
name string
blob *blob.Blob
// want is the expected tx response ABCI code.
want uint32
}
testCases := []testCase{
{
name: "2 mebibyte blob",
blob: newBlobWithSize(2 * mebibyte),
want: blobtypes.ErrBlobsTooLarge.ABCICode(),
},
}

signer, err := testnode.NewSignerFromContext(s.cctx, s.accounts[0])
require.NoError(t, err)

for _, tc := range testCases {
s.Run(tc.name, func() {
subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), 30*time.Second)
defer cancel()
res, err := signer.SubmitPayForBlob(subCtx, []*blob.Blob{tc.blob}, user.SetGasLimitAndFee(1e9, appconsts.DefaultGlobalMinGasPrice))
require.Error(t, err)
require.NotNil(t, res)
require.Equal(t, tc.want, res.Code, res.Logs)
})
}
}
43 changes: 33 additions & 10 deletions app/test/priority_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package app_test
import (
"encoding/hex"
"sort"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -70,8 +71,13 @@ func (s *PriorityTestSuite) TestPriorityByGasPrice() {
t := s.T()

// quickly submit blobs with a random fee
hashes := make([]string, 0, len(s.signers))

hashes := make(chan string, len(s.signers))
blobSize := uint32(100)
gasLimit := blobtypes.DefaultEstimateGas([]uint32{blobSize})
wg := &sync.WaitGroup{}
for _, signer := range s.signers {
<<<<<<< HEAD
blobSize := uint32(100)
gasLimit := blobtypes.DefaultEstimateGas([]uint32{blobSize})
gasPrice := s.rand.Float64()
Expand All @@ -88,25 +94,42 @@ func (s *PriorityTestSuite) TestPriorityByGasPrice() {
require.NoError(t, err)
require.Equal(t, abci.CodeTypeOK, resp.Code)
hashes = append(hashes, resp.TxHash)
=======
wg.Add(1)
go func() {
defer wg.Done()
gasPrice := float64(s.rand.Intn(1000)+1) / 1000
resp, err := signer.SubmitPayForBlob(
s.cctx.GoContext(),
blobfactory.ManyBlobs(
s.rand,
[]namespace.Namespace{namespace.RandomBlobNamespace()},
[]int{100}),
user.SetGasLimitAndFee(gasLimit, gasPrice),
)
require.NoError(t, err)
require.Equal(t, abci.CodeTypeOK, resp.Code, resp.RawLog)
hashes <- resp.TxHash
}()
>>>>>>> deefb542 (feat: nonce handling with signer (#3196))
}

wg.Wait()
close(hashes)

err := s.cctx.WaitForNextBlock()
require.NoError(t, err)

// get the responses for each tx for analysis and sort by height
// note: use rpc types because they contain the tx index
heightMap := make(map[int64][]*rpctypes.ResultTx)
for _, hash := range hashes {
resp, err := s.signers[0].ConfirmTx(s.cctx.GoContext(), hash)
require.NoError(t, err)
require.NotNil(t, resp)
require.Equal(t, abci.CodeTypeOK, resp.Code)
for hash := range hashes {
// use the core rpc type because it contains the tx index
hash, err := hex.DecodeString(hash)
require.NoError(t, err)
coreRes, err := s.cctx.Client.Tx(s.cctx.GoContext(), hash, false)
require.NoError(t, err)
heightMap[resp.Height] = append(heightMap[resp.Height], coreRes)
heightMap[coreRes.Height] = append(heightMap[coreRes.Height], coreRes)
}
require.GreaterOrEqual(t, len(heightMap), 1)

Expand All @@ -123,7 +146,7 @@ func (s *PriorityTestSuite) TestPriorityByGasPrice() {

// check that there was at least one block with more than three transactions
// in it. This is more of a sanity check than a test.
require.True(t, highestNumOfTxsPerBlock > 3)
require.Greater(t, highestNumOfTxsPerBlock, 3)
}

func sortByIndex(txs []*rpctypes.ResultTx) []*rpctypes.ResultTx {
Expand All @@ -135,14 +158,14 @@ func sortByIndex(txs []*rpctypes.ResultTx) []*rpctypes.ResultTx {

func isSortedByFee(t *testing.T, ecfg encoding.Config, responses []*rpctypes.ResultTx) bool {
for i := 0; i < len(responses)-1; i++ {
if gasPrice(t, ecfg, responses[i]) <= gasPrice(t, ecfg, responses[i+1]) {
if getGasPrice(t, ecfg, responses[i]) <= getGasPrice(t, ecfg, responses[i+1]) {
return false
}
}
return true
}

func gasPrice(t *testing.T, ecfg encoding.Config, resp *rpctypes.ResultTx) float64 {
func getGasPrice(t *testing.T, ecfg encoding.Config, resp *rpctypes.ResultTx) float64 {
sdkTx, err := ecfg.TxConfig.TxDecoder()(resp.Tx)
require.NoError(t, err)
feeTx := sdkTx.(sdk.FeeTx)
Expand Down
Loading

0 comments on commit 1930c89

Please sign in to comment.