From e0a0e030a7123d70eac4655f1b51e1a5163a77d3 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Fri, 9 Oct 2020 15:43:17 -0700 Subject: [PATCH 1/9] Add helper to predict entry IDs for claimable balances --- txnbuild/transaction.go | 59 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index c38e24e4a5..1efdc9e90b 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -346,6 +346,65 @@ func (t *Transaction) Base64() (string, error) { return marshallBase64(t.envelope, t.signatures) } +// BalanceIDs returns a list of balance IDs for all possible CreateClaimableBalance +// operations within this transaction. +func (t *Transaction) BalanceIDs() []string { + var balances []string + for opIndex, operation := range t.operations { + creation, ok := operation.(*CreateClaimableBalance) + if !ok { + continue + } + + // Use the operation's source account or the transaction's source if not. + var account Account + if creation.SourceAccount == nil { + account = &t.sourceAccount + } else { + account = creation.GetSourceAccount() + } + + seq, err := account.GetSequenceNumber() + if err != nil { + panic(errors.Wrap(err, "failed to query account sequence num")) + } + + // We mimic the relevant code from Stellar Core + // https://github.com/stellar/stellar-core/blob/9f3cc04e6ec02c38974c42545a86cdc79809252b/src/test/TestAccount.cpp#L285 + operationId := xdr.OperationId{ + Type: xdr.EnvelopeTypeEnvelopeTypeOpId, + Id: &xdr.OperationIdId{ + SourceAccount: xdr.MustMuxedAddress(account.GetAccountID()), + SeqNum: xdr.SequenceNumber(seq), + OpNum: xdr.Uint32(opIndex), + }, + } + + binaryDump, err := operationId.MarshalBinary() + if err != nil { + panic(errors.Wrap(err, "invalid claimable balance operation")) + } + + hash := sha256.Sum256(binaryDump) + balanceIdXdr, err := xdr.NewClaimableBalanceId( + // TODO: Determine the type programmatically + xdr.ClaimableBalanceIdTypeClaimableBalanceIdTypeV0, + xdr.Hash(hash)) + if err != nil { + panic(errors.Wrap(err, "unable to parse balance ID as XDR")) + } + + balanceIdHex, err := xdr.MarshalHex(balanceIdXdr) + if err != nil { + panic(errors.Wrap(err, "unable to parse balance ID as hex")) + } + + balances = append(balances, balanceIdHex) + } + + return balances +} + // FeeBumpTransaction represents a CAP 15 fee bump transaction. // Fee bump transactions allow an arbitrary account to pay the fee for a transaction. type FeeBumpTransaction struct { From 68711d55f21729c46a25b398f248dd0b7bfe230b Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Fri, 9 Oct 2020 15:43:34 -0700 Subject: [PATCH 2/9] Add integration test helpers to build raw txs conveniently --- .../internal/test/integration/integration.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/services/horizon/internal/test/integration/integration.go b/services/horizon/internal/test/integration/integration.go index 748889323a..2c0abbe676 100644 --- a/services/horizon/internal/test/integration/integration.go +++ b/services/horizon/internal/test/integration/integration.go @@ -569,6 +569,16 @@ func (i *Test) SubmitOperations( func (i *Test) SubmitMultiSigOperations( source txnbuild.Account, signers []*keypair.Full, ops ...txnbuild.Operation, ) (proto.Transaction, error) { + tx, err := i.CreateSignedTransaction(source, signers, ops...) + if err != nil { + return proto.Transaction{}, err + } + return i.SubmitTransaction(tx) +} + +func (i *Test) CreateSignedTransaction( + source txnbuild.Account, signers []*keypair.Full, ops ...txnbuild.Operation, +) (*txnbuild.Transaction, error) { txParams := txnbuild.TransactionParams{ SourceAccount: source, Operations: ops, @@ -579,16 +589,20 @@ func (i *Test) SubmitMultiSigOperations( tx, err := txnbuild.NewTransaction(txParams) if err != nil { - return proto.Transaction{}, err + return tx, err } for _, signer := range signers { tx, err = tx.Sign(NetworkPassphrase, signer) if err != nil { - return proto.Transaction{}, err + return tx, err } } + return tx, nil +} + +func (i *Test) SubmitTransaction(tx *txnbuild.Transaction) (proto.Transaction, error) { txb64, err := tx.Base64() if err != nil { return proto.Transaction{}, err From cc988e1e4dae593e5941f292c3956480f878643c Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Fri, 9 Oct 2020 15:43:46 -0700 Subject: [PATCH 3/9] Add test to ensure predictions match reality for balance IDs --- .../internal/integration/protocol14_test.go | 70 +++++++++++++++---- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/services/horizon/internal/integration/protocol14_test.go b/services/horizon/internal/integration/protocol14_test.go index 5934a661aa..e42a701fef 100644 --- a/services/horizon/internal/integration/protocol14_test.go +++ b/services/horizon/internal/integration/protocol14_test.go @@ -7,6 +7,7 @@ import ( "time" sdk "github.com/stellar/go/clients/horizonclient" + "github.com/stellar/go/keypair" proto "github.com/stellar/go/protocols/horizon" "github.com/stellar/go/protocols/horizon/operations" "github.com/stellar/go/services/horizon/internal/codes" @@ -20,25 +21,66 @@ var protocol14Config = integration.Config{ProtocolVersion: 14} func TestProtocol14Basics(t *testing.T) { tt := assert.New(t) - itest := integration.NewTest(t, protocol14Config) master := itest.Master() - root, err := itest.Client().Root() - tt.NoError(err) - tt.Equal(int32(14), root.CoreSupportedProtocolVersion) - tt.Equal(int32(14), root.CurrentProtocolVersion) + t.Run("Sanity", func(t *testing.T) { + root, err := itest.Client().Root() + tt.NoError(err) + tt.Equal(int32(14), root.CoreSupportedProtocolVersion) + tt.Equal(int32(14), root.CurrentProtocolVersion) + + // Submit a simple tx + op := txnbuild.Payment{ + Destination: master.Address(), + Amount: "10", + Asset: txnbuild.NativeAsset{}, + } - // Submit a simple tx - op := txnbuild.Payment{ - Destination: master.Address(), - Amount: "10", - Asset: txnbuild.NativeAsset{}, - } + txResp := itest.MustSubmitOperations(itest.MasterAccount(), master, &op) + tt.Equal(master.Address(), txResp.Account) + tt.Equal("1", txResp.AccountSequence) + }) + + // Ensure predicting claimable balances works. + t.Run("BalanceIDs", func(t *testing.T) { + tx, err := itest.CreateSignedTransaction( + itest.MasterAccount(), + []*keypair.Full{master}, + &txnbuild.CreateClaimableBalance{ + Destinations: []txnbuild.Claimant{ + txnbuild.NewClaimant(master.Address(), nil), + }, + Asset: txnbuild.NativeAsset{}, + Amount: "42", + }, + &txnbuild.CreateClaimableBalance{ + Destinations: []txnbuild.Claimant{ + txnbuild.NewClaimant(master.Address(), nil), + }, + Asset: txnbuild.NativeAsset{}, + Amount: "24", + }) + tt.NoError(err) - txResp := itest.MustSubmitOperations(itest.MasterAccount(), master, &op) - tt.Equal(master.Address(), txResp.Account) - tt.Equal("1", txResp.AccountSequence) + predictions := tx.BalanceIDs() + tt.Len(predictions, 2) + + var txResult xdr.TransactionResult + txResp, err := itest.SubmitTransaction(tx) + tt.NoError(err) + xdr.SafeUnmarshalBase64(txResp.ResultXdr, &txResult) + opResults, ok := txResult.OperationResults() + tt.True(ok) + tt.Len(opResults, len(predictions)) + + for i, predictedId := range predictions { + claimCreationOp := opResults[i].MustTr().CreateClaimableBalanceResult + calculatedId, err := xdr.MarshalHex(claimCreationOp.BalanceId) + tt.NoError(err) + tt.Equal(predictedId, calculatedId) + } + }) } func TestHappyClaimableBalances(t *testing.T) { From 91307f2177dcfcd812e7eaaf374b83b6a09256d9 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Fri, 9 Oct 2020 15:57:49 -0700 Subject: [PATCH 4/9] Update CHANGELOG accordingly --- txnbuild/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/txnbuild/CHANGELOG.md b/txnbuild/CHANGELOG.md index 8da248d143..e429fd8782 100644 --- a/txnbuild/CHANGELOG.md +++ b/txnbuild/CHANGELOG.md @@ -6,6 +6,8 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased * Add helper function `ParseAssetString()`, making it easier to build an `Asset` structure from a string in [canonical form](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0011.md#asset) and check its various properties ([#3105](https://github.com/stellar/go/pull/3105)). +* Add helper function `Transaction.BalanceIDs()`, making it easier to calculate balance IDs for [claimable balances](https://developers.stellar.org/docs/glossary/claimable-balance/) without actually submitting the transaction ([#3122](https://github.com/stellar/go/pull/3122)). + ## [v4.0.1](https://github.com/stellar/go/releases/tag/horizonclient-v4.0.1) - 2020-10-02 * Fixed bug in `TransactionFromXDR()` which occurs when parsing transaction XDR envelopes which contain Protocol 14 operations. From d9b9d7088aab10133d2af2a136ee15a4892fcca2 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Tue, 13 Oct 2020 12:17:32 -0700 Subject: [PATCH 5/9] Refactor helper function to accept an operation index parameter. This is a cleaner design than returning a list of IDs, because it actually enforces a correlation between the returned ID and a specific operation. Otherwise, it's strange to have the returned array be different that the list of operations in the transaction. --- txnbuild/CHANGELOG.md | 2 +- txnbuild/transaction.go | 95 ++++++++++++++++++++--------------------- 2 files changed, 48 insertions(+), 49 deletions(-) diff --git a/txnbuild/CHANGELOG.md b/txnbuild/CHANGELOG.md index e429fd8782..4c2eb21ae5 100644 --- a/txnbuild/CHANGELOG.md +++ b/txnbuild/CHANGELOG.md @@ -6,7 +6,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased * Add helper function `ParseAssetString()`, making it easier to build an `Asset` structure from a string in [canonical form](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0011.md#asset) and check its various properties ([#3105](https://github.com/stellar/go/pull/3105)). -* Add helper function `Transaction.BalanceIDs()`, making it easier to calculate balance IDs for [claimable balances](https://developers.stellar.org/docs/glossary/claimable-balance/) without actually submitting the transaction ([#3122](https://github.com/stellar/go/pull/3122)). +* Add helper function `Transaction.ClaimableBalanceID()`, making it easier to calculate balance IDs for [claimable balances](https://developers.stellar.org/docs/glossary/claimable-balance/) without actually submitting the transaction ([#3122](https://github.com/stellar/go/pull/3122)). ## [v4.0.1](https://github.com/stellar/go/releases/tag/horizonclient-v4.0.1) - 2020-10-02 diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 1efdc9e90b..80a46cecf4 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -346,63 +346,62 @@ func (t *Transaction) Base64() (string, error) { return marshallBase64(t.envelope, t.signatures) } -// BalanceIDs returns a list of balance IDs for all possible CreateClaimableBalance -// operations within this transaction. -func (t *Transaction) BalanceIDs() []string { - var balances []string - for opIndex, operation := range t.operations { - creation, ok := operation.(*CreateClaimableBalance) - if !ok { - continue - } +// Returns the (pre-calculated) claimable balance ID for the operation at the +// given index (which should be a `CreateClaimableBalance` operation). +func (t *Transaction) ClaimableBalanceID(operationIndex int) (string, error) { + if operationIndex < 0 || operationIndex >= len(t.operations) { + return "", errors.New("invalid operation index") + } - // Use the operation's source account or the transaction's source if not. - var account Account - if creation.SourceAccount == nil { - account = &t.sourceAccount - } else { - account = creation.GetSourceAccount() - } + operation, ok := t.operations[operationIndex].(*CreateClaimableBalance) + if !ok { + return "", errors.New("operation is not CreateClaimableBalance") + } - seq, err := account.GetSequenceNumber() - if err != nil { - panic(errors.Wrap(err, "failed to query account sequence num")) - } + // Use the operation's source account or the transaction's source if not. + var account Account + if operation.SourceAccount == nil { + account = &t.sourceAccount + } else { + account = operation.GetSourceAccount() + } - // We mimic the relevant code from Stellar Core - // https://github.com/stellar/stellar-core/blob/9f3cc04e6ec02c38974c42545a86cdc79809252b/src/test/TestAccount.cpp#L285 - operationId := xdr.OperationId{ - Type: xdr.EnvelopeTypeEnvelopeTypeOpId, - Id: &xdr.OperationIdId{ - SourceAccount: xdr.MustMuxedAddress(account.GetAccountID()), - SeqNum: xdr.SequenceNumber(seq), - OpNum: xdr.Uint32(opIndex), - }, - } + seq, err := account.GetSequenceNumber() + if err != nil { + return "", errors.Wrap(err, "failed to retrieve account sequence number") + } - binaryDump, err := operationId.MarshalBinary() - if err != nil { - panic(errors.Wrap(err, "invalid claimable balance operation")) - } + // We mimic the relevant code from Stellar Core + // https://github.com/stellar/stellar-core/blob/9f3cc04e6ec02c38974c42545a86cdc79809252b/src/test/TestAccount.cpp#L285 + operationId := xdr.OperationId{ + Type: xdr.EnvelopeTypeEnvelopeTypeOpId, + Id: &xdr.OperationIdId{ + SourceAccount: xdr.MustMuxedAddress(account.GetAccountID()), + SeqNum: xdr.SequenceNumber(seq), + OpNum: xdr.Uint32(operationIndex), + }, + } - hash := sha256.Sum256(binaryDump) - balanceIdXdr, err := xdr.NewClaimableBalanceId( - // TODO: Determine the type programmatically - xdr.ClaimableBalanceIdTypeClaimableBalanceIdTypeV0, - xdr.Hash(hash)) - if err != nil { - panic(errors.Wrap(err, "unable to parse balance ID as XDR")) - } + binaryDump, err := operationId.MarshalBinary() + if err != nil { + return "", errors.Wrap(err, "invalid claimable balance operation") + } - balanceIdHex, err := xdr.MarshalHex(balanceIdXdr) - if err != nil { - panic(errors.Wrap(err, "unable to parse balance ID as hex")) - } + hash := sha256.Sum256(binaryDump) + balanceIdXdr, err := xdr.NewClaimableBalanceId( + // Can this be determined programmatically from the operation structure? + xdr.ClaimableBalanceIdTypeClaimableBalanceIdTypeV0, + xdr.Hash(hash)) + if err != nil { + return "", errors.Wrap(err, "unable to parse balance ID as XDR") + } - balances = append(balances, balanceIdHex) + balanceIdHex, err := xdr.MarshalHex(balanceIdXdr) + if err != nil { + return "", errors.Wrap(err, "unable to encode balance ID as hex") } - return balances + return balanceIdHex, nil } // FeeBumpTransaction represents a CAP 15 fee bump transaction. From ede3b89d6818d520f5369ef2b8f5a8d47f45ac70 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Tue, 13 Oct 2020 12:19:29 -0700 Subject: [PATCH 6/9] Update test to use new helper properly --- services/horizon/internal/integration/protocol14_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/services/horizon/internal/integration/protocol14_test.go b/services/horizon/internal/integration/protocol14_test.go index e42a701fef..ba9f63feac 100644 --- a/services/horizon/internal/integration/protocol14_test.go +++ b/services/horizon/internal/integration/protocol14_test.go @@ -63,9 +63,12 @@ func TestProtocol14Basics(t *testing.T) { }) tt.NoError(err) - predictions := tx.BalanceIDs() - tt.Len(predictions, 2) - + id1, err := tx.ClaimableBalanceID(0) + tt.NoError(err) + id2, err := tx.ClaimableBalanceID(1) + tt.NoError(err) + predictions := []string{id1, id2} + var txResult xdr.TransactionResult txResp, err := itest.SubmitTransaction(tx) tt.NoError(err) From 8c84d77e2dde4b130f9ecad7c178b438ed462168 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Tue, 13 Oct 2020 12:38:16 -0700 Subject: [PATCH 7/9] really? --- services/horizon/internal/integration/protocol14_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/horizon/internal/integration/protocol14_test.go b/services/horizon/internal/integration/protocol14_test.go index ba9f63feac..9931f2d199 100644 --- a/services/horizon/internal/integration/protocol14_test.go +++ b/services/horizon/internal/integration/protocol14_test.go @@ -68,7 +68,7 @@ func TestProtocol14Basics(t *testing.T) { id2, err := tx.ClaimableBalanceID(1) tt.NoError(err) predictions := []string{id1, id2} - + var txResult xdr.TransactionResult txResp, err := itest.SubmitTransaction(tx) tt.NoError(err) From ac0c86b943e605981141769a80186294e10b2390 Mon Sep 17 00:00:00 2001 From: George Date: Wed, 14 Oct 2020 14:26:46 -0700 Subject: [PATCH 8/9] Integrate code review suggestions Docstrings and return values are cleaned up Co-authored-by: Howard Tinghao Chen Co-authored-by: Leigh McCulloch --- .../horizon/internal/test/integration/integration.go | 4 ++-- txnbuild/transaction.go | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/services/horizon/internal/test/integration/integration.go b/services/horizon/internal/test/integration/integration.go index 2c0abbe676..a8613b089b 100644 --- a/services/horizon/internal/test/integration/integration.go +++ b/services/horizon/internal/test/integration/integration.go @@ -589,13 +589,13 @@ func (i *Test) CreateSignedTransaction( tx, err := txnbuild.NewTransaction(txParams) if err != nil { - return tx, err + return nil, err } for _, signer := range signers { tx, err = tx.Sign(NetworkPassphrase, signer) if err != nil { - return tx, err + return nil, err } } diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 80a46cecf4..4d54819446 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -346,7 +346,7 @@ func (t *Transaction) Base64() (string, error) { return marshallBase64(t.envelope, t.signatures) } -// Returns the (pre-calculated) claimable balance ID for the operation at the +// ClaimableBalanceID returns the claimable balance ID for the operation at the given index within the transaction. // given index (which should be a `CreateClaimableBalance` operation). func (t *Transaction) ClaimableBalanceID(operationIndex int) (string, error) { if operationIndex < 0 || operationIndex >= len(t.operations) { @@ -359,10 +359,8 @@ func (t *Transaction) ClaimableBalanceID(operationIndex int) (string, error) { } // Use the operation's source account or the transaction's source if not. - var account Account - if operation.SourceAccount == nil { - account = &t.sourceAccount - } else { + account := &t.sourceAccount + if operation.SourceAccount != nil { account = operation.GetSourceAccount() } @@ -389,7 +387,7 @@ func (t *Transaction) ClaimableBalanceID(operationIndex int) (string, error) { hash := sha256.Sum256(binaryDump) balanceIdXdr, err := xdr.NewClaimableBalanceId( - // Can this be determined programmatically from the operation structure? + // TODO: look into whether this be determined programmatically from the operation structure. xdr.ClaimableBalanceIdTypeClaimableBalanceIdTypeV0, xdr.Hash(hash)) if err != nil { From d6c303ca2f64473f6269c3bcf3a5a496151e02a8 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Wed, 14 Oct 2020 14:32:46 -0700 Subject: [PATCH 9/9] Use proper interface type in account variable setup --- txnbuild/transaction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 4d54819446..194dbae817 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -359,7 +359,7 @@ func (t *Transaction) ClaimableBalanceID(operationIndex int) (string, error) { } // Use the operation's source account or the transaction's source if not. - account := &t.sourceAccount + var account Account = &t.sourceAccount if operation.SourceAccount != nil { account = operation.GetSourceAccount() }