Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Number Precision - Use Repo.CurrencyValue to represent amounts uniformly #1666

Merged
merged 42 commits into from
Aug 28, 2019
Merged

Number Precision - Use Repo.CurrencyValue to represent amounts uniformly #1666

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
c32f13a
update balance field
amangale Jul 23, 2019
935adcf
bring in the vendor changes
amangale Jul 28, 2019
dd3b8ce
bring in the vendor changes
amangale Jul 28, 2019
387793c
bring in test changes
amangale Jul 28, 2019
760c300
bring in util changes
amangale Jul 28, 2019
c684f4b
bring in the repo changes
amangale Jul 28, 2019
f6c22eb
pull in api changes
amangale Jul 28, 2019
341e373
pull in cmd changes
amangale Jul 28, 2019
cf6d142
pull in repo changes
amangale Jul 28, 2019
0b0d0a6
pull in repo changes
amangale Jul 28, 2019
3d6e3af
pull in qa changes
amangale Jul 28, 2019
25b89c2
pull in pb changes
amangale Jul 28, 2019
51633f7
pull in ipfs changes
amangale Jul 28, 2019
de55a78
pull in api changes
amangale Jul 28, 2019
593ba6f
pull in handler changes
amangale Jul 28, 2019
c844aaf
pull in pb and repo changes
amangale Jul 28, 2019
9036c4c
pull in core changes
amangale Jul 28, 2019
439a517
update dependencies
amangale Jul 28, 2019
f36ba97
fix tests
amangale Jul 29, 2019
7dfde53
shift migrations
amangale Aug 6, 2019
f0d4ffd
resolve conflicts and re-order migrations
amangale Aug 6, 2019
b8ba4b4
Rebase to master
cpacia Aug 12, 2019
d6e5f60
fix review comments - err handling
amangale Aug 16, 2019
88aed5c
fix review comments - migrations and comments
amangale Aug 16, 2019
8b5146b
fix review comments
amangale Aug 16, 2019
4eeee5f
fix review comments
amangale Aug 16, 2019
795cc80
merge master
amangale Aug 19, 2019
503ebc4
fix unitspercoin
amangale Aug 19, 2019
b5d3c5a
Update QA tests to fix dictionary and json errors
cpacia Aug 19, 2019
45227b7
fix disputes accuracy return and price modifier calculation
amangale Aug 20, 2019
48227c4
Merge remote-tracking branch 'origin/master' into numprec
placer14 Aug 20, 2019
7d8ffb6
qa fix crypto purchase test
amangale Aug 21, 2019
ad9abd6
qa fix update listing and smtp tests
amangale Aug 22, 2019
24dc0f6
Tighten linter errcheck; Fix outstanding linter errors
placer14 Aug 22, 2019
b54f136
Update qa test num blocks generated
cpacia Aug 23, 2019
bde38a6
Merge branch 'numprec' of https://github.com/OpenBazaar/openbazaar-go…
cpacia Aug 23, 2019
88c6690
Fix repo/db/keys_test to use isolated databases
placer14 Aug 23, 2019
4063627
Fix amount formatting in failing qa tests
cpacia Aug 23, 2019
5eda8ca
Merge branch 'numprec' of https://github.com/OpenBazaar/openbazaar-go…
cpacia Aug 23, 2019
98e6cae
Add `make qa` command; Remove qa/test_framework compilation artifacts
placer14 Aug 23, 2019
865ac43
Update golangci-lint --new-from-rev to SHA 24dc0f64
placer14 Aug 23, 2019
c080927
Fix bugs calcualatig output values in DisputeClose
cpacia Aug 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
204 changes: 122 additions & 82 deletions api/jsonapi.go
View file
Open in desktop

Large diffs are not rendered by default.

93 changes: 82 additions & 11 deletions api/jsonapi_data_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ const moderatorValidJSON = `{
"fee": {
"feeType": "FIXED_PLUS_PERCENTAGE",
"fixedFee": {
"currencyCode": "USD",
"amount": 300
"currency": {code: "USD", divisibility: 8},
"amount": "300"
},
"percentage": 5
}
Expand All @@ -331,24 +331,88 @@ const walletAddressJSONResponse = `{

const walletBalanceJSONResponse = `{
"TBCH": {
"confirmed": 0,
"confirmed": {
"amount": "0",
"currency": {
"code": "TBCH",
"currencyType": "crypto",
"divisibility": 8,
"name": "Bitcoin Cash"
}
},
"height": 0,
"unconfirmed": 0
"unconfirmed": {
"amount": "0",
"currency": {
"code": "TBCH",
"currencyType": "crypto",
"divisibility": 8,
"name": "Bitcoin Cash"
}
}
},
"TBTC": {
"confirmed": 0,
"confirmed": {
"amount": "0",
"currency": {
"code": "TBTC",
"currencyType": "crypto",
"divisibility": 8,
"name": "Bitcoin"
}
},
"height": 0,
"unconfirmed": 0
"unconfirmed": {
"amount": "0",
"currency": {
"code": "TBTC",
"currencyType": "crypto",
"divisibility": 8,
"name": "Bitcoin"
}
}
},
"TLTC": {
"confirmed": 0,
"confirmed": {
"amount": "0",
"currency": {
"code": "TLTC",
"currencyType": "crypto",
"divisibility": 8,
"name": "Litecoin"
}
},
"height": 0,
"unconfirmed": 0
"unconfirmed": {
"amount": "0",
"currency": {
"code": "TLTC",
"currencyType": "crypto",
"divisibility": 8,
"name": "Litecoin"
}
}
},
"TZEC": {
"confirmed": 0,
"confirmed": {
"amount": "0",
"currency": {
"code": "TZEC",
"currencyType": "crypto",
"divisibility": 8,
"name": "Zcash"
}
},
"height": 0,
"unconfirmed": 0
"unconfirmed": {
"amount": "0",
"currency": {
"code": "TZEC",
"currencyType": "crypto",
"divisibility": 8,
"name": "Zcash"
}
}
}
}`

Expand All @@ -359,7 +423,14 @@ const walletBalanceJSONResponse = `{
const spendJSON = `{
"wallet": "btc",
"address": "1HYhu8e2wv19LZ2umXoo1pMiwzy2rL32UQ",
"amount": 1700000,
"amount": {
placer14 marked this conversation as resolved.
Show resolved Hide resolved
"amount": "1700000",
"currency": {
"code": "BTC",
"currencyType": "crypto",
"civisibility": 8
}
},
"feeLevel": "NORMAL"
}`

Expand Down
53 changes: 32 additions & 21 deletions api/jsonapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,31 +404,35 @@ func TestCryptoListingsQuantity(t *testing.T) {
})
}

/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's up with this blocked out test?

func TestCryptoListingsNoCoinType(t *testing.T) {
listing := factory.NewCryptoListing("crypto")
listing.Metadata.CoinType = ""
//listing.Metadata.CoinType = ""

runAPITests(t, apiTests{
{"POST", "/ob/listing", jsonFor(t, listing), 500, errorResponseJSON(core.ErrCryptocurrencyListingCoinTypeRequired)},
})
}
*/

/*
func TestCryptoListingsCoinDivisibilityIncorrect(t *testing.T) {
listing := factory.NewCryptoListing("crypto")
runAPITests(t, apiTests{
{"POST", "/ob/listing", jsonFor(t, listing), 200, anyResponseJSON},
})

listing.Metadata.CoinDivisibility = 1e7
//listing.Metadata.CoinDivisibility = 1e7
runAPITests(t, apiTests{
{"POST", "/ob/listing", jsonFor(t, listing), 500, errorResponseJSON(core.ErrListingCoinDivisibilityIncorrect)},
})

listing.Metadata.CoinDivisibility = 0
//listing.Metadata.CoinDivisibility = 0
runAPITests(t, apiTests{
{"POST", "/ob/listing", jsonFor(t, listing), 500, errorResponseJSON(core.ErrListingCoinDivisibilityIncorrect)},
})
}
*/

func TestCryptoListingsIllegalFields(t *testing.T) {
runTest := func(listing *pb.Listing, err error) {
Expand All @@ -439,11 +443,11 @@ func TestCryptoListingsIllegalFields(t *testing.T) {

physicalListing := factory.NewListing("physical")

listing := factory.NewCryptoListing("crypto")
listing.Metadata.PricingCurrency = "btc"
runTest(listing, core.ErrCryptocurrencyListingIllegalField("metadata.pricingCurrency"))
//listing := factory.NewCryptoListing("crypto")
//listing.Metadata.PricingCurrency = &pb.CurrencyDefinition{Code: "BTC", Divisibility: 8}
//runTest(listing, core.ErrCryptocurrencyListingIllegalField("metadata.pricingCurrency"))

listing = factory.NewCryptoListing("crypto")
listing := factory.NewCryptoListing("crypto")
listing.Item.Condition = "new"
runTest(listing, core.ErrCryptocurrencyListingIllegalField("item.condition"))

Expand All @@ -457,13 +461,20 @@ func TestCryptoListingsIllegalFields(t *testing.T) {

listing = factory.NewCryptoListing("crypto")
listing.Coupons = physicalListing.Coupons
/*[]*pb.Listing_Coupon{}
sampleCoupon := new(pb.Listing_Coupon)
sampleCoupon.Title = "sample coupon"
sampleCoupon.Code = &pb.Listing_Coupon_DiscountCode{DiscountCode: "insider"}
sampleCoupon.Discount = &pb.Listing_Coupon_PercentDiscount{PercentDiscount: 5}
*/
runTest(listing, core.ErrCryptocurrencyListingIllegalField("coupons"))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to verify the following repo.Listing PR handles these tests.

}

func TestMarketRatePrice(t *testing.T) {
listing := factory.NewListing("listing")
listing.Metadata.Format = pb.Listing_Metadata_MARKET_PRICE
listing.Item.Price = 1
listing.Item.PriceValue = &pb.CurrencyValue{Currency: &pb.CurrencyDefinition{Code: "BTC", Divisibility: 8}, Amount: "100"}

runAPITests(t, apiTests{
{"POST", "/ob/listing", jsonFor(t, listing), 500, errorResponseJSON(core.ErrMarketPriceListingIllegalField("item.price"))},
Expand Down Expand Up @@ -570,7 +581,7 @@ func TestCloseDisputeBlocksWhenExpired(t *testing.T) {
func TestZECSalesCannotReleaseEscrow(t *testing.T) {
sale := factory.NewSaleRecord()
sale.Contract.VendorListings[0].Metadata.AcceptedCurrencies = []string{"ZEC"}
sale.Contract.BuyerOrder.Payment.Coin = "ZEC"
sale.Contract.BuyerOrder.Payment.AmountValue = &pb.CurrencyValue{Currency: &pb.CurrencyDefinition{Code: "ZEC", Divisibility: 8}}
dbSetup := func(testRepo *test.Repository) error {
if err := testRepo.DB.Sales().Put(sale.OrderID, *sale.Contract, sale.OrderState, false); err != nil {
return err
Expand All @@ -585,7 +596,7 @@ func TestZECSalesCannotReleaseEscrow(t *testing.T) {
func TestSalesGet(t *testing.T) {
sale := factory.NewSaleRecord()
sale.Contract.VendorListings[0].Metadata.AcceptedCurrencies = []string{"BTC"}
sale.Contract.VendorListings[0].Metadata.CoinType = "ZEC"
//sale.Contract.VendorListings[0].Metadata.CoinType = "ZEC"
sale.Contract.VendorListings[0].Metadata.ContractType = pb.Listing_Metadata_CRYPTOCURRENCY
dbSetup := func(testRepo *test.Repository) error {
return testRepo.DB.Sales().Put(sale.OrderID, *sale.Contract, sale.OrderState, false)
Expand Down Expand Up @@ -615,9 +626,9 @@ func TestSalesGet(t *testing.T) {
if actualSale.BuyerId != sale.Contract.BuyerOrder.BuyerID.PeerID {
t.Fatal("Incorrect buyerId:", actualSale.BuyerId, "\nwanted:", sale.Contract.BuyerOrder.BuyerID.PeerID)
}
if actualSale.CoinType != sale.Contract.VendorListings[0].Metadata.CoinType {
t.Fatal("Incorrect coinType:", actualSale.CoinType, "\nwanted:", sale.Contract.VendorListings[0].Metadata.CoinType)
}
//if actualSale.CoinType != sale.Contract.VendorListings[0].Metadata.CoinType {
// t.Fatal("Incorrect coinType:", actualSale.CoinType, "\nwanted:", sale.Contract.VendorListings[0].Metadata.CoinType)
//}
if actualSale.OrderId != sale.OrderID {
t.Fatal("Incorrect orderId:", actualSale.OrderId, "\nwanted:", sale.OrderID)
}
Expand All @@ -637,7 +648,7 @@ func TestSalesGet(t *testing.T) {
func TestPurchasesGet(t *testing.T) {
purchase := factory.NewPurchaseRecord()
purchase.Contract.VendorListings[0].Metadata.AcceptedCurrencies = []string{"BTC"}
purchase.Contract.VendorListings[0].Metadata.CoinType = "ZEC"
//purchase.Contract.VendorListings[0].Metadata.CoinType = "ZEC"
purchase.Contract.VendorListings[0].Metadata.ContractType = pb.Listing_Metadata_CRYPTOCURRENCY
dbSetup := func(testRepo *test.Repository) error {
return testRepo.DB.Purchases().Put(purchase.OrderID, *purchase.Contract, purchase.OrderState, false)
Expand Down Expand Up @@ -667,9 +678,9 @@ func TestPurchasesGet(t *testing.T) {
if actualPurchase.VendorId != purchase.Contract.VendorListings[0].VendorID.PeerID {
t.Fatal("Incorrect vendorId:", actualPurchase.VendorId, "\nwanted:", purchase.Contract.VendorListings[0].VendorID.PeerID)
}
if actualPurchase.CoinType != purchase.Contract.VendorListings[0].Metadata.CoinType {
t.Fatal("Incorrect coinType:", actualPurchase.CoinType, "\nwanted:", purchase.Contract.VendorListings[0].Metadata.CoinType)
}
//if actualPurchase.CoinType != purchase.Contract.VendorListings[0].Metadata.CoinType {
// t.Fatal("Incorrect coinType:", actualPurchase.CoinType, "\nwanted:", purchase.Contract.VendorListings[0].Metadata.CoinType)
//}
if actualPurchase.OrderId != purchase.OrderID {
t.Fatal("Incorrect orderId:", actualPurchase.OrderId, "\nwanted:", purchase.OrderID)
}
Expand All @@ -691,7 +702,7 @@ func TestCasesGet(t *testing.T) {
paymentCoinCode := repo.CurrencyCode("BTC")
disputeCaseRecord := factory.NewDisputeCaseRecord()
disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.AcceptedCurrencies = []string{"BTC"}
disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.CoinType = "ZEC"
//disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.CoinType = "ZEC"
disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.ContractType = pb.Listing_Metadata_CRYPTOCURRENCY
disputeCaseRecord.CoinType = "ZEC"
disputeCaseRecord.PaymentCoin = &paymentCoinCode
Expand All @@ -717,9 +728,9 @@ func TestCasesGet(t *testing.T) {

actualCase := respObj.Cases[0]

if actualCase.CoinType != disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.CoinType {
t.Fatal("Incorrect coinType:", actualCase.CoinType, "\nwanted:", disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.CoinType)
}
//if actualCase.CoinType != disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.CoinType {
// t.Fatal("Incorrect coinType:", actualCase.CoinType, "\nwanted:", disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.CoinType)
//}
if actualCase.PaymentCoin != disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.AcceptedCurrencies[0] {
t.Fatal("Incorrect paymentCoin:", actualCase.PaymentCoin, "\nwanted:", disputeCaseRecord.BuyerContract.VendorListings[0].Metadata.AcceptedCurrencies[0])
}
Expand Down
4 changes: 4 additions & 0 deletions cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,10 @@ func (x *Start) Execute(args []string) error {
if err != nil {
return err
}
} else if x.Regtest {
if r, ok := router.(*dht.IpfsDHT); ok {
dhtRouting = r
}
}
}
if dhtRouting == nil {
Expand Down
29 changes: 16 additions & 13 deletions core/completion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (
"fmt"
libp2p "gx/ipfs/QmTW4SdgBWq9GjsBsHeUx8WuGxzhgzAf88UMH2w62PC8yK/go-libp2p-crypto"
"io/ioutil"
"math/big"
"os"
"path"
"strings"
"time"

"github.com/OpenBazaar/jsonpb"
Expand Down Expand Up @@ -162,26 +164,26 @@ func (n *OpenBazaarNode) CompleteOrder(orderRatings *OrderRatings, contract *pb.
oc.Ratings = append(oc.Ratings, rating)
}

wal, err := n.Multiwallet.WalletForCurrencyCode(contract.BuyerOrder.Payment.Coin)
wal, err := n.Multiwallet.WalletForCurrencyCode(contract.BuyerOrder.Payment.AmountValue.Currency.Code)
if err != nil {
return err
}

// Payout order if moderated and not disputed
if contract.BuyerOrder.Payment.Method == pb.Order_Payment_MODERATED && contract.DisputeResolution == nil {
var ins []wallet.TransactionInput
var outValue int64
outValue := new(big.Int)
for _, r := range records {
if !r.Spent && r.Value > 0 {
if !r.Spent && r.Value.Cmp(big.NewInt(0)) > 0 {
addr, err := wal.DecodeAddress(r.Address)
if err != nil {
return err
}
outpointHash, err := hex.DecodeString(r.Txid)
outpointHash, err := hex.DecodeString(strings.TrimPrefix(r.Txid, "0x"))
if err != nil {
return fmt.Errorf("decoding transaction hash: %s", err.Error())
}
outValue += r.Value
outValue.Add(outValue, &r.Value)
in := wallet.TransactionInput{
LinkedAddress: addr,
OutpointIndex: r.Index,
Expand All @@ -198,7 +200,7 @@ func (n *OpenBazaarNode) CompleteOrder(orderRatings *OrderRatings, contract *pb.
}
var output = wallet.TransactionOutput{
Address: payoutAddress,
Value: outValue,
Value: *outValue,
}

chaincode, err := hex.DecodeString(contract.BuyerOrder.Payment.Chaincode)
Expand All @@ -217,8 +219,8 @@ func (n *OpenBazaarNode) CompleteOrder(orderRatings *OrderRatings, contract *pb.
if err != nil {
return err
}

buyerSignatures, err := wal.CreateMultisigSignature(ins, []wallet.TransactionOutput{output}, buyerKey, redeemScript, contract.VendorOrderFulfillment[0].Payout.PayoutFeePerByte)
n, _ := new(big.Int).SetString(contract.VendorOrderFulfillment[0].Payout.PayoutFeePerByteValue, 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle this error with a meaningful return.

buyerSignatures, err := wal.CreateMultisigSignature(ins, []wallet.TransactionOutput{output}, buyerKey, redeemScript, *n)
if err != nil {
return err
}
Expand All @@ -235,7 +237,8 @@ func (n *OpenBazaarNode) CompleteOrder(orderRatings *OrderRatings, contract *pb.
sig := wallet.Signature{InputIndex: s.InputIndex, Signature: s.Signature}
vendorSignatures = append(vendorSignatures, sig)
}
_, err = wal.Multisign(ins, []wallet.TransactionOutput{output}, buyerSignatures, vendorSignatures, redeemScript, contract.VendorOrderFulfillment[0].Payout.PayoutFeePerByte, true)
//n, _ = new(big.Int).SetString(contract.VendorOrderFulfillment[0].Payout.PayoutFeePerByte, 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete unused code.

_, err = wal.Multisign(ins, []wallet.TransactionOutput{output}, buyerSignatures, vendorSignatures, redeemScript, *n, true)
if err != nil {
return err
}
Expand Down Expand Up @@ -307,16 +310,16 @@ func (n *OpenBazaarNode) ReleaseFundsAfterTimeout(contract *pb.RicardianContract
} else if active {
return ErrPrematureReleaseOfTimedoutEscrowFunds
}
wal, err := n.Multiwallet.WalletForCurrencyCode(contract.BuyerOrder.Payment.Coin)
wal, err := n.Multiwallet.WalletForCurrencyCode(contract.BuyerOrder.Payment.AmountValue.Currency.Code)
if err != nil {
return err
}

minConfirms := contract.VendorListings[0].Metadata.EscrowTimeoutHours * ConfirmationsPerHour
var txInputs []wallet.TransactionInput
for _, r := range records {
if !r.Spent && r.Value > 0 {
hash, err := chainhash.NewHashFromStr(r.Txid)
if !r.Spent && r.Value.Cmp(big.NewInt(0)) > 0 {
hash, err := chainhash.NewHashFromStr(strings.TrimPrefix(r.Txid, "0x"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is messy. Some places the 0x is okay and some places it's not. Plus, this might hurt a txid which legitimately has that starting prefix. How can we keep this concern completely contained within the ETH wallet?

if err != nil {
return err
}
Expand All @@ -335,7 +338,7 @@ func (n *OpenBazaarNode) ReleaseFundsAfterTimeout(contract *pb.RicardianContract
if err != nil {
return err
}
outpointHash, err := hex.DecodeString(r.Txid)
outpointHash, err := hex.DecodeString(strings.TrimPrefix(r.Txid, "0x"))
if err != nil {
return fmt.Errorf("decoding transaction hash: %s", err.Error())
}
Expand Down
Loading