Skip to content

Commit

Permalink
protocols/horizon: Change Account.Sequence to int64 (#4438)
Browse files Browse the repository at this point in the history
Change `protocols/horizon/Account.Sequence` to `int64` from `string`.

There shouldn't be any extra parsing required in Go since `Account.Sequence`
returned by Horizon will be always a valid int64 value. We can make sure the
value is rendered as a string to support JS using `string` tag.

Similar to: #4409
  • Loading branch information
bartekn authored Jun 24, 2022
1 parent b29a93b commit 79683d8
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 94 deletions.
21 changes: 6 additions & 15 deletions protocols/horizon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type Account struct {

ID string `json:"id"`
AccountID string `json:"account_id"`
Sequence string `json:"sequence"`
Sequence int64 `json:"sequence,string"`
SequenceLedger uint32 `json:"sequence_ledger,omitempty"`
SequenceTime string `json:"sequence_time,omitempty"`
SubentryCount int32 `json:"subentry_count"`
Expand Down Expand Up @@ -98,29 +98,20 @@ func (a Account) GetCreditBalance(code string, issuer string) string {

// GetSequenceNumber returns the sequence number of the account,
// and returns it as a 64-bit integer.
// TODO: since Account.Sequence was changed to int64, error is no longer needed.
func (a Account) GetSequenceNumber() (int64, error) {
seqNum, err := strconv.ParseInt(a.Sequence, 10, 64)
if err != nil {
return 0, errors.Wrap(err, "Failed to parse account sequence number")
}

return seqNum, nil
return a.Sequence, nil
}

// IncrementSequenceNumber increments the internal record of the account's sequence
// number by 1. This is typically used after a transaction build so that the next
// transaction to be built will be valid.
func (a *Account) IncrementSequenceNumber() (int64, error) {
seqNum, err := a.GetSequenceNumber()
if err != nil {
return 0, err
}
if seqNum == math.MaxInt64 {
if a.Sequence == math.MaxInt64 {
return 0, fmt.Errorf("sequence cannot be increased, it already reached MaxInt64 (%d)", int64(math.MaxInt64))
}
seqNum++
a.Sequence = strconv.FormatInt(seqNum, 10)
return seqNum, nil
a.Sequence++
return a.Sequence, nil
}

// MustGetData returns decoded value for a given key. If the key does
Expand Down
4 changes: 2 additions & 2 deletions protocols/horizon/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ var exampleAccount = Account{
"test": "aGVsbG8=",
"invalid": "a_*&^*",
},
Sequence: "3002985298788353",
Sequence: 3002985298788353,
}

func TestAccount_IncrementSequenceNumber(t *testing.T) {
seqNum, err := exampleAccount.IncrementSequenceNumber()

assert.Nil(t, err)
assert.Equal(t, "3002985298788354", exampleAccount.Sequence, "sequence number string was incremented")
assert.Equal(t, int64(3002985298788354), exampleAccount.Sequence, "sequence number was incremented")
assert.Equal(t, int64(3002985298788354), seqNum, "incremented sequence number is correct value/type")
}

Expand Down
4 changes: 2 additions & 2 deletions services/friendbot/init_friendbot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestInitFriendbot_createMinionAccounts_success(t *testing.T) {
botAccountID := botKeypair.Address()
botAccountMock := horizon.Account{
AccountID: botAccountID,
Sequence: "1",
Sequence: 1,
}
botAccount := internal.Account{AccountID: botAccountID, Sequence: 1}

Expand Down Expand Up @@ -55,7 +55,7 @@ func TestInitFriendbot_createMinionAccounts_timeoutError(t *testing.T) {
botAccountID := botKeypair.Address()
botAccountMock := horizon.Account{
AccountID: botAccountID,
Sequence: "1",
Sequence: 1,
}
botAccount := internal.Account{AccountID: botAccountID, Sequence: 1}

Expand Down
8 changes: 1 addition & 7 deletions services/friendbot/internal/account.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package internal

import (
"strconv"

"github.com/stellar/go/clients/horizonclient"
"github.com/stellar/go/support/errors"
)
Expand Down Expand Up @@ -36,10 +34,6 @@ func (a *Account) RefreshSequenceNumber(hclient horizonclient.ClientInterface) e
if err != nil {
return errors.Wrap(err, "getting account detail")
}
seq, err := strconv.ParseInt(accountDetail.Sequence, 10, 64)
if err != nil {
return errors.Wrap(err, "parsing account seqnum")
}
a.Sequence = seq
a.Sequence = accountDetail.Sequence
return nil
}
2 changes: 1 addition & 1 deletion services/horizon/internal/actions/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func TestAccountInfo(t *testing.T) {
account, err := AccountInfo(tt.Ctx, &history.Q{tt.HorizonSession()}, accountID)
tt.Assert.NoError(err)

tt.Assert.Equal("8589934593", account.Sequence)
tt.Assert.Equal(int64(8589934593), account.Sequence)
tt.Assert.Equal(uint32(4), account.LastModifiedLedger)
tt.Assert.NotNil(account.LastModifiedTime)
tt.Assert.Equal(ledgerFourCloseTime, account.LastModifiedTime.Unix())
Expand Down
4 changes: 1 addition & 3 deletions services/horizon/internal/integration/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,10 @@ func submitAccountOps(itest *integration.Test, tt *assert.Assertions) (submitted
allOps := ops
itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(), ops...)
account := itest.MustGetAccount(accountPair)
seq, err := strconv.ParseInt(account.Sequence, 10, 64)
tt.NoError(err)
domain := "www.example.com"
ops = []txnbuild.Operation{
&txnbuild.BumpSequence{
BumpTo: seq + 1000,
BumpTo: account.Sequence + 1000,
},
&txnbuild.SetOptions{
HomeDomain: &domain,
Expand Down
3 changes: 1 addition & 2 deletions services/horizon/internal/resourceadapter/account_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package resourceadapter
import (
"context"
"fmt"
"strconv"

protocol "github.com/stellar/go/protocols/horizon"
horizonContext "github.com/stellar/go/services/horizon/internal/context"
Expand All @@ -26,7 +25,7 @@ func PopulateAccountEntry(
dest.ID = account.AccountID
dest.PT = account.AccountID
dest.AccountID = account.AccountID
dest.Sequence = strconv.FormatInt(account.SequenceNumber, 10)
dest.Sequence = account.SequenceNumber
if account.SequenceLedger.Valid && account.SequenceTime.Valid {
dest.SequenceLedger = uint32(account.SequenceLedger.Int64)
dest.SequenceTime = fmt.Sprintf("%d", account.SequenceTime.Int64)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -146,7 +145,7 @@ func TestPopulateAccountEntry(t *testing.T) {
tt.Equal(account.AccountID, hAccount.ID)
tt.Equal(account.AccountID, hAccount.AccountID)
tt.Equal(account.AccountID, hAccount.PT)
tt.Equal(strconv.FormatInt(account.SequenceNumber, 10), hAccount.Sequence)
tt.Equal(account.SequenceNumber, hAccount.Sequence)
tt.Equal(uint32(account.SequenceLedger.Int64), hAccount.SequenceLedger)
tt.Equal(fmt.Sprintf("%d", account.SequenceTime.Int64), hAccount.SequenceTime)
tt.Equal(account.NumSubEntries, uint32(hAccount.SubentryCount))
Expand Down
8 changes: 2 additions & 6 deletions services/horizon/internal/test/integration/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,17 +575,13 @@ func (i *Test) CreateAccounts(count int, initialBalance string) ([]*keypair.Full
// Two paths here: either caller already did some stuff with the master
// account so we should retrieve the sequence number, or caller hasn't and
// we start from scratch.
seq := int64(0)
request := sdk.AccountRequest{AccountID: master.Address()}
account, err := client.AccountDetail(request)
if err == nil {
seq, err = strconv.ParseInt(account.Sequence, 10, 64) // str -> bigint
panicIf(err)
}
panicIf(err)

masterAccount := txnbuild.SimpleAccount{
AccountID: master.Address(),
Sequence: seq,
Sequence: account.Sequence,
}

for i := 0; i < count; i++ {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestSetup_accountAlreadyConfigured(t *testing.T) {
AuthRevocable: true,
},
HomeDomain: "domain.test.com",
Sequence: "10",
Sequence: 10,
}, nil)
horizonMock.
On("Assets", horizonclient.AssetRequest{
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestSetup(t *testing.T) {
On("AccountDetail", horizonclient.AccountRequest{AccountID: issuerKP.Address()}).
Return(horizon.Account{
AccountID: issuerKP.Address(),
Sequence: "10",
Sequence: 10,
}, nil)
horizonMock.
On("Assets", horizonclient.AssetRequest{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestAPI_txApprove_revised(t *testing.T) {
On("AccountDetail", horizonclient.AccountRequest{AccountID: senderKP.Address()}).
Return(horizon.Account{
AccountID: senderKP.Address(),
Sequence: "5",
Sequence: 5,
}, nil)

handler := txApproveHandler{
Expand All @@ -103,7 +103,7 @@ func TestAPI_txApprove_revised(t *testing.T) {
txnbuild.TransactionParams{
SourceAccount: &horizon.Account{
AccountID: senderKP.Address(),
Sequence: "5",
Sequence: 5,
},
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestAPI_txAprove_actionRequired(t *testing.T) {
On("AccountDetail", horizonclient.AccountRequest{AccountID: senderKP.Address()}).
Return(horizon.Account{
AccountID: senderKP.Address(),
Sequence: "1",
Sequence: 1,
}, nil)

handler := txApproveHandler{
Expand All @@ -223,7 +223,7 @@ func TestAPI_txAprove_actionRequired(t *testing.T) {
txnbuild.TransactionParams{
SourceAccount: &horizon.Account{
AccountID: senderKP.Address(),
Sequence: "1",
Sequence: 1,
},
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{
Expand Down Expand Up @@ -293,7 +293,7 @@ func TestAPI_txAprove_actionRequiredFlow(t *testing.T) {
On("AccountDetail", horizonclient.AccountRequest{AccountID: senderKP.Address()}).
Return(horizon.Account{
AccountID: senderKP.Address(),
Sequence: "1",
Sequence: 1,
}, nil)

handler := txApproveHandler{
Expand All @@ -316,7 +316,7 @@ func TestAPI_txAprove_actionRequiredFlow(t *testing.T) {
txnbuild.TransactionParams{
SourceAccount: &horizon.Account{
AccountID: senderKP.Address(),
Sequence: "1",
Sequence: 1,
},
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{
Expand Down Expand Up @@ -485,7 +485,7 @@ func TestAPI_txApprove_success(t *testing.T) {
On("AccountDetail", horizonclient.AccountRequest{AccountID: senderKP.Address()}).
Return(horizon.Account{
AccountID: senderKP.Address(),
Sequence: "5",
Sequence: 5,
}, nil)

handler := txApproveHandler{
Expand All @@ -504,7 +504,7 @@ func TestAPI_txApprove_success(t *testing.T) {
tx, err := txnbuild.NewTransaction(txnbuild.TransactionParams{
SourceAccount: &horizon.Account{
AccountID: senderKP.Address(),
Sequence: "5",
Sequence: 5,
},
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func TestFriendbotHandler_serveHTTP(t *testing.T) {
On("AccountDetail", horizonclient.AccountRequest{AccountID: "GDDIO6SFRD4SJEQFJOSKPIDYTDM7LM4METFBKN4NFGVR5DTGB7H75N5S"}).
Return(horizon.Account{
AccountID: "GDDIO6SFRD4SJEQFJOSKPIDYTDM7LM4METFBKN4NFGVR5DTGB7H75N5S",
Sequence: "1",
Sequence: 1,
}, nil)
horizonMock.
On("SubmitTransaction", mock.AnythingOfType("*txnbuild.Transaction")).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,8 @@ func (h txApproveHandler) txApprove(ctx context.Context, in txApproveRequest) (r
}

// validate the sequence number
accountSequence, err := strconv.ParseInt(acc.Sequence, 10, 64)
if err != nil {
return nil, errors.Wrapf(err, "parsing account sequence number %q from string to int64", acc.Sequence)
}
if tx.SourceAccount().Sequence != accountSequence+1 {
log.Ctx(ctx).Errorf(`invalid transaction sequence number tx.SourceAccount().Sequence: %d, accountSequence+1: %d`, tx.SourceAccount().Sequence, accountSequence+1)
if tx.SourceAccount().Sequence != acc.Sequence+1 {
log.Ctx(ctx).Errorf(`invalid transaction sequence number tx.SourceAccount().Sequence: %d, accountSequence+1: %d`, tx.SourceAccount().Sequence, acc.Sequence+1)
return NewRejectedTxApprovalResponse("Invalid transaction sequence number."), nil
}

Expand Down Expand Up @@ -328,12 +324,8 @@ func (h txApproveHandler) handleSuccessResponseIfNeeded(ctx context.Context, tx
if err != nil {
return nil, errors.Wrapf(err, "getting detail for payment source account %s", paymentSource)
}
accountSequence, err := strconv.ParseInt(acc.Sequence, 10, 64)
if err != nil {
return nil, errors.Wrapf(err, "parsing account sequence number %q from string to int64", acc.Sequence)
}
if tx.SourceAccount().Sequence != accountSequence+1 {
log.Ctx(ctx).Errorf(`invalid transaction sequence number tx.SourceAccount().Sequence: %d, accountSequence+1: %d`, tx.SourceAccount().Sequence, accountSequence+1)
if tx.SourceAccount().Sequence != acc.Sequence+1 {
log.Ctx(ctx).Errorf(`invalid transaction sequence number tx.SourceAccount().Sequence: %d, accountSequence+1: %d`, tx.SourceAccount().Sequence, acc.Sequence+1)
return NewRejectedTxApprovalResponse("Invalid transaction sequence number."), nil
}

Expand Down
Loading

0 comments on commit 79683d8

Please sign in to comment.