Skip to content

Commit

Permalink
services/horizon: Pass through nil ExtraSigners to avoid nil pointer …
Browse files Browse the repository at this point in the history 
…deref (#4349)
  • Loading branch information
@Shaptic
Shaptic authored Apr 21, 2022
1 parent 18ab1d8 commit ec61581
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 28 deletions.
3 changes: 1 addition & 2 deletions services/horizon/internal/db2/history/fee_bump_scenario.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

sq "github.com/Masterminds/squirrel"
"github.com/guregu/null"
"github.com/lib/pq"

"github.com/stellar/go/ingest"
"github.com/stellar/go/network"
Expand Down Expand Up @@ -314,7 +313,7 @@ func FeeBumpScenario(tt *test.T, q *Q, successful bool) FeeBumpFixture {
Memo: null.NewString("", false),
TimeBounds: TimeBounds{Lower: null.IntFrom(2), Upper: null.IntFrom(4)},
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Signatures: signatures(fixture.Envelope.FeeBumpSignatures()),
InnerSignatures: signatures(fixture.Envelope.Signatures()),
Successful: successful,
Expand Down
4 changes: 4 additions & 0 deletions services/horizon/internal/db2/history/transaction_batch_insert_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ func formatUint32(u *xdr.Uint32) null.Int {
}

func formatSigners(s []xdr.SignerKey) pq.StringArray {
if s == nil {
return nil
}

signers := make([]string, len(s))
for i, key := range s {
signers[i] = key.Address()
Expand Down
27 changes: 13 additions & 14 deletions services/horizon/internal/db2/history/transaction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

sq "github.com/Masterminds/squirrel"
"github.com/guregu/null"
"github.com/lib/pq"
"github.com/stellar/go/xdr"

"github.com/stellar/go/ingest"
Expand Down Expand Up @@ -364,7 +363,7 @@ func TestInsertTransaction(t *testing.T) {
Successful: success,
TimeBounds: nullTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
},
},
},
Expand Down Expand Up @@ -400,7 +399,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("", false),
TimeBounds: nullTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -437,7 +436,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("", false),
TimeBounds: nullTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: false,
},
},
Expand Down Expand Up @@ -475,7 +474,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("test memo", true),
TimeBounds: infiniteTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -512,7 +511,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("test memo", true),
TimeBounds: infiniteTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -549,7 +548,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("123", true),
TimeBounds: nullTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -586,7 +585,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("fi3vINWiGla+KkV7ZI9wLuGviJ099leQ6SoFCB6fq/E=", true),
TimeBounds: infiniteTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -623,7 +622,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("zdjArlILa/LNv7o7lo/qv5+fVVPNl0yPgZQWB6u+gL4=", true),
TimeBounds: infiniteTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -661,7 +660,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("", false),
TimeBounds: timeBoundWithMin,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -698,7 +697,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("", false),
TimeBounds: timeBoundWithMax,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -735,7 +734,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("", false),
TimeBounds: timeboundsWithMinAndMax,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -778,7 +777,7 @@ func TestInsertTransaction(t *testing.T) {
MinAccountSequence: null.Int{},
MinAccountSequenceAge: null.StringFrom("10"),
MinAccountSequenceLedgerGap: null.IntFrom(2),
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down Expand Up @@ -815,7 +814,7 @@ func TestInsertTransaction(t *testing.T) {
Memo: null.NewString("", false),
TimeBounds: nullTimeBounds,
LedgerBounds: LedgerBounds{Null: true},
ExtraSigners: pq.StringArray{},
ExtraSigners: nil,
Successful: success,
},
},
Expand Down
78 changes: 77 additions & 1 deletion services/horizon/internal/integration/transaction_preconditions_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package integration

import (
"bytes"
"encoding/base64"
"fmt"
"strconv"
"sync"
Expand All @@ -9,9 +11,11 @@ import (

sdk "github.com/stellar/go/clients/horizonclient"
"github.com/stellar/go/keypair"
"github.com/stellar/go/network"
"github.com/stellar/go/protocols/horizon"
"github.com/stellar/go/services/horizon/internal/test/integration"
"github.com/stellar/go/txnbuild"
"github.com/stellar/go/xdr"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -286,7 +290,6 @@ func TestTransactionPreconditionsMinSequenceNumberLedgerGap(t *testing.T) {
}

func buildTXParams(master *keypair.Full, masterAccount txnbuild.Account, txSequence int64) txnbuild.TransactionParams {

return txnbuild.TransactionParams{
SourceAccount: &txnbuild.SimpleAccount{
AccountID: masterAccount.GetAccountID(),
Expand Down Expand Up @@ -334,3 +337,76 @@ func TestTransactionPreconditionsAccountV3Fields(t *testing.T) {
tt.Equal(uint32(tx.Ledger), account.SequenceLedger)
tt.Equal(strconv.FormatInt(tx.LedgerCloseTime.Unix(), 10), account.SequenceTime)
}

// TestTransactionWithoutPreconditions ensures that Horizon doesn't break when
// we have a PRECOND_NONE type transaction (which is not possible to submit
// through SDKs, but is absolutely still possible).
func TestTransactionWithoutPreconditions(t *testing.T) {
tt := assert.New(t)
itest := integration.NewTest(t, integration.Config{})
if itest.GetEffectiveProtocolVersion() < 19 {
t.Skip("Can't run with protocol < 19")
}

master := itest.Master()
masterAccount := itest.MasterAccount()
seqNum, err := masterAccount.GetSequenceNumber()
tt.NoError(err)

account := xdr.MuxedAccount{}
tt.NoError(account.SetEd25519Address(master.Address()))

payment := txnbuild.Payment{ // dummy op
Destination: master.Address(),
Amount: "1000",
Asset: txnbuild.NativeAsset{},
}
paymentOp, err := payment.BuildXDR()
tt.NoError(err)

envelope := xdr.TransactionEnvelope{
Type: xdr.EnvelopeTypeEnvelopeTypeTx,
V1: &xdr.TransactionV1Envelope{
Tx: xdr.Transaction{
SourceAccount: account,
Fee: xdr.Uint32(1000),
SeqNum: xdr.SequenceNumber(seqNum + 1),
Operations: []xdr.Operation{paymentOp},
Cond: xdr.Preconditions{
Type: xdr.PreconditionTypePrecondNone,
},
},
Signatures: nil,
},
}

// Taken from txnbuild.concatSignatures
h, err := network.HashTransactionInEnvelope(envelope,
integration.StandaloneNetworkPassphrase)
tt.NoError(err)

sig, err := master.SignDecorated(h[:])
tt.NoError(err)

// taken from txnbuild.marshallBinary
var txBytes bytes.Buffer
envelope.V1.Signatures = []xdr.DecoratedSignature{sig}
_, err = xdr.Marshal(&txBytes, envelope)
tt.NoError(err)
b64 := base64.StdEncoding.EncodeToString(txBytes.Bytes())

txResp, err := itest.Client().SubmitTransactionXDR(b64)
tt.NoError(err)

fmt.Println(
"envelopeXDR", txResp.EnvelopeXdr,
"resultXDR", txResp.ResultXdr,
// "feeChangesXDR", txResp.feeChangesXDR,
"metaXDR", txResp.FeeMetaXdr,
"hash", txResp.Hash,
)

txResp2, err := itest.Client().TransactionDetail(txResp.Hash)
tt.NoError(err)
tt.Nil(txResp2.Preconditions)
}
24 changes: 13 additions & 11 deletions services/horizon/internal/resourceadapter/transaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,23 @@ func PopulateTransaction(
}
dest.Signatures = row.Signatures

if row.HasPreconditions() {
dest.Preconditions = &protocol.TransactionPreconditions{}
}
// If we never use this, we'll remove it later. This just defends us against
// nil dereferences.
dest.Preconditions = &protocol.TransactionPreconditions{}

if !row.TimeBounds.Null {
// Action needed in release: horizon-v3.0.0: remove ValidBefore and ValidAfter
dest.ValidBefore = timeString(row.TimeBounds.Upper)
dest.ValidAfter = timeString(row.TimeBounds.Lower)

if dest.Preconditions.TimeBounds == nil {
dest.Preconditions.TimeBounds = &protocol.TransactionPreconditionsTimebounds{}
dest.Preconditions.TimeBounds = &protocol.TransactionPreconditionsTimebounds{
MaxTime: timeString(row.TimeBounds.Upper),
MinTime: timeString(row.TimeBounds.Lower),
}
dest.Preconditions.TimeBounds.MaxTime = timeString(row.TimeBounds.Upper)
dest.Preconditions.TimeBounds.MinTime = timeString(row.TimeBounds.Lower)
}

if !row.LedgerBounds.Null {
if dest.Preconditions.LedgerBounds == nil {
dest.Preconditions.LedgerBounds = &protocol.TransactionPreconditionsLedgerbounds{}
}
dest.Preconditions.LedgerBounds = &protocol.TransactionPreconditionsLedgerbounds{}
if row.LedgerBounds.MinLedger.Valid {
dest.Preconditions.LedgerBounds.MinLedger = uint32(row.LedgerBounds.MinLedger.Int64)
}
Expand All @@ -95,7 +92,7 @@ func PopulateTransaction(
dest.Preconditions.MinAccountSequenceLedgerGap = uint32(row.MinAccountSequenceLedgerGap.Int64)
}

if row.ExtraSigners != nil {
if len(row.ExtraSigners) > 0 {
dest.Preconditions.ExtraSigners = row.ExtraSigners
}

Expand Down Expand Up @@ -136,6 +133,11 @@ func PopulateTransaction(
dest.Links.Succeeds = lb.Linkf("/transactions?order=desc&cursor=%s", dest.PT)
dest.Links.Precedes = lb.Linkf("/transactions?order=asc&cursor=%s", dest.PT)

// If we didn't need the structure, drop it.
if !row.HasPreconditions() {
dest.Preconditions = nil
}

return nil
}

Expand Down

0 comments on commit ec61581

Please sign in to comment.