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

txnbuild: Use xdr.Price to represent prices in txnbuild instead of strings. #4167

Merged
merged 3 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions price/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ func Parse(v string) (xdr.Price, error) {
return continuedFraction(v)
}

// MustParse is like Parse except that it panics on errors.
func MustParse(v string) xdr.Price {
result, err := Parse(v)
if err != nil {
panic(err)
}
return result
}

// continuedFraction calculates and returns the best rational approximation of
// the given real number.
func continuedFraction(price string) (xdrPrice xdr.Price, err error) {
Expand Down
4 changes: 2 additions & 2 deletions services/horizon/internal/integration/clawback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestHappyClawbackAccountSellingLiabilities(t *testing.T) {
Buying: txnbuild.NativeAsset{},
Selling: asset,
Amount: "5",
Price: "1",
Price: xdr.Price{1, 1},
SourceAccount: fromAccount.GetAccountID(),
})
tt.True(submissionResp.Successful)
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestSadClawbackAccountSufficientFundsSellingLiabilities(t *testing.T) {
Buying: txnbuild.NativeAsset{},
Selling: asset,
Amount: "5",
Price: "1",
Price: xdr.Price{1, 1},
SourceAccount: fromAccount.GetAccountID(),
})

Expand Down
4 changes: 2 additions & 2 deletions services/horizon/internal/integration/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ func generateLiquidityPoolOps(itest *integration.Test, tt *assert.Assertions) (l
LiquidityPoolID: [32]byte(poolID),
MaxAmountA: "400",
MaxAmountB: "777",
MinPrice: "0.5",
MaxPrice: "2",
MinPrice: xdr.Price{1, 2},
MaxPrice: xdr.Price{2, 1},
},
)

Expand Down
12 changes: 6 additions & 6 deletions services/horizon/internal/integration/liquidity_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ func TestLiquidityPoolHappyPath(t *testing.T) {
LiquidityPoolID: [32]byte(poolID),
MaxAmountA: "400",
MaxAmountB: "777",
MinPrice: "0.5",
MaxPrice: "2",
MinPrice: xdr.Price{1, 2},
MaxPrice: xdr.Price{2, 1},
},
)

Expand Down Expand Up @@ -475,8 +475,8 @@ func TestLiquidityPoolRevoke(t *testing.T) {
LiquidityPoolID: [32]byte(poolID),
MaxAmountA: "400",
MaxAmountB: "777",
MinPrice: "0.5",
MaxPrice: "2",
MinPrice: xdr.Price{1, 2},
MaxPrice: xdr.Price{2, 1},
},
&txnbuild.SetTrustLineFlags{
SourceAccount: master.Address(),
Expand Down Expand Up @@ -667,8 +667,8 @@ func TestLiquidityPoolFailedDepositAndWithdraw(t *testing.T) {
LiquidityPoolID: nonExistentPoolID,
MaxAmountA: "400",
MaxAmountB: "777",
MinPrice: "0.5",
MaxPrice: "2",
MinPrice: xdr.Price{1, 2},
MaxPrice: xdr.Price{2, 1},
},
)
_, err = itest.Client().SubmitTransaction(tx)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ func TestMuxedOperations(t *testing.T) {
Selling: txnbuild.NativeAsset{},
Buying: txnbuild.CreditAsset{"ABCD", master.Address()},
Amount: "3",
Price: "1",
Price: xdr.Price{1, 1},
},
// This will generate a trade effect:
&txnbuild.ManageSellOffer{
SourceAccount: masterMuxed.Address(),
Selling: txnbuild.CreditAsset{"ABCD", master.Address()},
Buying: txnbuild.NativeAsset{},
Amount: "3",
Price: "1",
Price: xdr.Price{1, 1},
},
&txnbuild.ManageData{
SourceAccount: sponsoredMuxed.Address(),
Expand Down
2 changes: 1 addition & 1 deletion services/horizon/internal/integration/sponsorship_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ func TestSponsorships(t *testing.T) {
Selling: txnbuild.NativeAsset{},
Buying: asset,
Amount: "3",
Price: "1",
Price: xdr.Price{1, 1},
})

signers := []*keypair.Full{sponsorPair, newAccountPair}
Expand Down
3 changes: 2 additions & 1 deletion services/horizon/internal/integration/state_verifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/stellar/go/services/horizon/internal/db2/history"
"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 @@ -63,7 +64,7 @@ func TestStateVerifier(t *testing.T) {
Selling: txnbuild.NativeAsset{},
Buying: txnbuild.CreditAsset{"ABCD", master.Address()},
Amount: "3",
Price: "1",
Price: xdr.Price{1, 1},
},
&txnbuild.ManageData{
SourceAccount: sponsoredSource.AccountID,
Expand Down
14 changes: 3 additions & 11 deletions txnbuild/create_passive_offer.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ type CreatePassiveSellOffer struct {
Selling Asset
Buying Asset
Amount string
Price string
price price
Price xdr.Price
SourceAccount string
}

Expand All @@ -34,15 +33,11 @@ func (cpo *CreatePassiveSellOffer) BuildXDR(withMuxedAccounts bool) (xdr.Operati
return xdr.Operation{}, errors.Wrap(err, "failed to parse 'Amount'")
}

if err = cpo.price.parse(cpo.Price); err != nil {
return xdr.Operation{}, errors.Wrap(err, "failed to parse 'Price'")
}

xdrOp := xdr.CreatePassiveSellOfferOp{
Selling: xdrSelling,
Buying: xdrBuying,
Amount: xdrAmount,
Price: cpo.price.toXDR(),
Price: cpo.Price,
}

opType := xdr.OperationTypeCreatePassiveSellOffer
Expand All @@ -68,10 +63,7 @@ func (cpo *CreatePassiveSellOffer) FromXDR(xdrOp xdr.Operation, withMuxedAccount

cpo.SourceAccount = accountFromXDR(xdrOp.SourceAccount, withMuxedAccounts)
cpo.Amount = amount.String(result.Amount)
if result.Price != (xdr.Price{}) {
cpo.price.fromXDR(result.Price)
cpo.Price = cpo.price.string()
}
cpo.Price = result.Price
buyingAsset, err := assetFromXDR(result.Buying)
if err != nil {
return errors.Wrap(err, "error parsing buying_asset in create_passive_sell_offer operation")
Expand Down
15 changes: 6 additions & 9 deletions txnbuild/create_passive_offer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestCreatePassiveSellOfferValidateBuyingAsset(t *testing.T) {
Selling: NativeAsset{},
Buying: CreditAsset{"ABCD", ""},
Amount: "10",
Price: "1.0",
Price: xdr.Price{1, 1},
}

_, err := NewTransaction(
Expand All @@ -42,7 +42,7 @@ func TestCreatePassiveSellOfferValidateSellingAsset(t *testing.T) {
Selling: CreditAsset{"ABCD0123456789", kp0.Address()},
Buying: NativeAsset{},
Amount: "10",
Price: "1.0",
Price: xdr.Price{1, 1},
}

_, err := NewTransaction(
Expand All @@ -69,7 +69,7 @@ func TestCreatePassiveSellOfferValidateAmount(t *testing.T) {
Selling: CreditAsset{"ABCD", kp0.Address()},
Buying: NativeAsset{},
Amount: "-3",
Price: "1.0",
Price: xdr.Price{1, 1},
}

_, err := NewTransaction(
Expand All @@ -96,7 +96,7 @@ func TestCreatePassiveSellOfferValidatePrice(t *testing.T) {
Selling: CreditAsset{"ABCD", kp0.Address()},
Buying: NativeAsset{},
Amount: "3",
Price: "-1.0",
Price: xdr.Price{-1, 0},
}

_, err := NewTransaction(
Expand All @@ -109,7 +109,7 @@ func TestCreatePassiveSellOfferValidatePrice(t *testing.T) {
},
)
if assert.Error(t, err) {
expected := `validation failed for *txnbuild.CreatePassiveSellOffer operation: Field: Price, Error: amount can not be negative`
expected := `validation failed for *txnbuild.CreatePassiveSellOffer operation: Field: Price, Error: price denominator cannot be 0: -1/0`
assert.Contains(t, err.Error(), expected)
}
}
Expand All @@ -121,19 +121,16 @@ func TestCreatePassiveSellOfferPrice(t *testing.T) {
Selling: CreditAsset{"ABCD", kp0.Address()},
Buying: NativeAsset{},
Amount: "1",
Price: "0.000000001",
Price: xdr.Price{1, 1000000000},
SourceAccount: kp0.Address(),
}

xdrOp, err := offer.BuildXDR(false)
assert.NoError(t, err)
expectedPrice := xdr.Price{N: 1, D: 1000000000}
assert.Equal(t, expectedPrice, xdrOp.Body.CreatePassiveSellOfferOp.Price)
assert.Equal(t, offer.Price, offer.price.string())
assert.Equal(t, expectedPrice, offer.price.toXDR())

parsed := CreatePassiveSellOffer{}
assert.NoError(t, parsed.FromXDR(xdrOp, false))
assert.Equal(t, offer.Price, parsed.Price)
assert.Equal(t, offer.price, parsed.price)
}
16 changes: 8 additions & 8 deletions txnbuild/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package txnbuild

import (
"fmt"
"github.com/stellar/go/price"
"github.com/stellar/go/xdr"
"time"

"github.com/stellar/go/keypair"
Expand Down Expand Up @@ -429,8 +431,7 @@ func ExampleManageSellOffer() {
selling := NativeAsset{}
buying := CreditAsset{"ABCD", "GAS4V4O2B7DW5T7IQRPEEVCRXMDZESKISR7DVIGKZQYYV3OSQ5SH5LVP"}
sellAmount := "100"
price := "0.01"
op, err := CreateOfferOp(selling, buying, sellAmount, price)
op, err := CreateOfferOp(selling, buying, sellAmount, price.MustParse("0.01"))
check(err)

tx, err := NewTransaction(
Expand Down Expand Up @@ -496,9 +497,8 @@ func ExampleManageSellOffer_updateOffer() {
selling := NativeAsset{}
buying := CreditAsset{"ABCD", "GAS4V4O2B7DW5T7IQRPEEVCRXMDZESKISR7DVIGKZQYYV3OSQ5SH5LVP"}
sellAmount := "50"
price := "0.02"
offerID := int64(2497628)
op, err := UpdateOfferOp(selling, buying, sellAmount, price, offerID)
op, err := UpdateOfferOp(selling, buying, sellAmount, price.MustParse("0.02"), offerID)
check(err)

tx, err := NewTransaction(
Expand Down Expand Up @@ -533,7 +533,7 @@ func ExampleCreatePassiveSellOffer() {
Selling: NativeAsset{},
Buying: CreditAsset{"ABCD", "GAS4V4O2B7DW5T7IQRPEEVCRXMDZESKISR7DVIGKZQYYV3OSQ5SH5LVP"},
Amount: "10",
Price: "1.0",
Price: xdr.Price{1, 1},
}

tx, err := NewTransaction(
Expand Down Expand Up @@ -682,7 +682,7 @@ func ExampleManageBuyOffer() {
Selling: NativeAsset{},
Buying: CreditAsset{"ABCD", "GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3"},
Amount: "100",
Price: "0.01",
Price: price.MustParse("0.01"),
OfferID: 0,
}

Expand Down Expand Up @@ -1040,8 +1040,8 @@ func ExampleLiquidityPoolDeposit() {
LiquidityPoolID: poolId,
MaxAmountA: "0.1000000",
MaxAmountB: "0.1000000",
MinPrice: "0.1000000",
MaxPrice: "0.1000000",
MinPrice: price.MustParse("0.1000000"),
MaxPrice: price.MustParse("0.1000000"),
},
}

Expand Down
19 changes: 16 additions & 3 deletions txnbuild/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ func validateAmount(n interface{}) error {
return nil
}

func validatePrice(p xdr.Price) error {
if p.N == 0 {
return errors.Errorf("price cannot be 0: %d/%d", p.N, p.D)
}
if p.D == 0 {
return errors.Errorf("price denominator cannot be 0: %d/%d", p.N, p.D)
}
if p.N < 0 || p.D < 0 {
return errors.Errorf("price cannot be negative: %d/%d", p.N, p.D)
}
return nil
}

// validateAssetCode checks if the provided asset is valid as an asset code.
// It returns an error if the asset is invalid.
// The asset must be non native (XLM) with a valid asset code.
Expand Down Expand Up @@ -141,7 +154,7 @@ func validateChangeTrustAsset(asset ChangeTrustAsset) error {
// validatePassiveOffer checks if the fields of a CreatePassiveOffer struct are valid.
// It checks that the buying and selling assets are valid stellar assets, and that amount and price are valid.
// It returns an error if any field is invalid.
func validatePassiveOffer(buying, selling Asset, offerAmount, price string) error {
func validatePassiveOffer(buying, selling Asset, offerAmount string, price xdr.Price) error {
// Note: see discussion on how this can be improved:
// https://github.com/stellar/go/pull/1707#discussion_r321508440
err := validateStellarAsset(buying)
Expand All @@ -159,7 +172,7 @@ func validatePassiveOffer(buying, selling Asset, offerAmount, price string) erro
return NewValidationError("Amount", err.Error())
}

err = validateAmount(price)
err = validatePrice(price)
if err != nil {
return NewValidationError("Price", err.Error())
}
Expand All @@ -170,7 +183,7 @@ func validatePassiveOffer(buying, selling Asset, offerAmount, price string) erro
// validateOffer checks if the fields of ManageBuyOffer or ManageSellOffer struct are valid.
// It checks that the buying and selling assets are valid stellar assets, and that amount, price and offerID
// are valid. It returns an error if any field is invalid.
func validateOffer(buying, selling Asset, offerAmount, price string, offerID int64) error {
func validateOffer(buying, selling Asset, offerAmount string, price xdr.Price, offerID int64) error {
err := validatePassiveOffer(buying, selling, offerAmount, price)
if err != nil {
return err
Expand Down
14 changes: 7 additions & 7 deletions txnbuild/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func TestValidatePassiveOfferInvalidAmount(t *testing.T) {
cpo := CreatePassiveSellOffer{
Buying: buying,
Selling: selling,
Price: "1",
Price: xdr.Price{1, 1},
Amount: "-1",
}
err := validatePassiveOffer(cpo.Buying, cpo.Selling, cpo.Amount, cpo.Price)
Expand All @@ -284,12 +284,12 @@ func TestValidatePassiveOfferInvalidPrice(t *testing.T) {
cpo := CreatePassiveSellOffer{
Buying: buying,
Selling: selling,
Price: "-1",
Price: xdr.Price{-1, 1},
Amount: "10",
}
err := validatePassiveOffer(cpo.Buying, cpo.Selling, cpo.Amount, cpo.Price)
assert.Error(t, err)
expectedErrMsg := "Field: Price, Error: amount can not be negative"
expectedErrMsg := "Field: Price, Error: price cannot be negative: -1/1"
require.EqualError(t, err, expectedErrMsg, "valid price is required")
}

Expand All @@ -299,7 +299,7 @@ func TestValidatePassiveOfferInvalidAsset(t *testing.T) {
cpo := CreatePassiveSellOffer{
Buying: buying,
Selling: selling,
Price: "1",
Price: xdr.Price{1, 1},
Amount: "10",
}
err := validatePassiveOffer(cpo.Buying, cpo.Selling, cpo.Amount, cpo.Price)
Expand All @@ -313,7 +313,7 @@ func TestValidatePassiveOfferInvalidAsset(t *testing.T) {
cpo1 := CreatePassiveSellOffer{
Buying: buying1,
Selling: selling1,
Price: "1",
Price: xdr.Price{1, 1},
Amount: "10",
}
err = validatePassiveOffer(cpo1.Buying, cpo1.Selling, cpo1.Amount, cpo1.Price)
Expand All @@ -329,7 +329,7 @@ func TestValidateOfferManageBuyOffer(t *testing.T) {
mbo := ManageBuyOffer{
Buying: buying,
Selling: selling,
Price: "1",
Price: xdr.Price{1, 1},
Amount: "10",
OfferID: -1,
}
Expand All @@ -346,7 +346,7 @@ func TestValidateOfferManageSellOffer(t *testing.T) {
mso := ManageSellOffer{
Buying: buying,
Selling: selling,
Price: "1",
Price: xdr.Price{1, 1},
Amount: "10",
OfferID: -1,
}
Expand Down
Loading