openbazaar-go icon indicating copy to clipboard operation
openbazaar-go copied to clipboard

Published 20 hours ago • verified publisher icon OpenBazaar

Improved Currency Precision

Open placer14 opened this issue 6 years ago • 3 comments

Today, the network assumes a certain precision when reading integer values of currency. OpenBazaar should allow each currency to be represented as an int with varying amounts of precision.

Considerations for BigInt representation (replace instances of json.Number in the Scope below with whatever solution we pick):

  • json.Number
  • https://github.com/shopspring/decimal

Scope

Protobuf Schema

  • create pb.CurrencyValue proto to track high-precision value of a currency
    • (smallest known precision is 1e-18)
proto CurrencyDefinition {
  string      code
  json.Number divisibility
}
proto CurrencyValue {
  CurrencyDefinition currency
  json.Number        value
}
  • restrict prior schema version to BTC, BCH, LTC, ZEC as accepted currencies for transactions (recommend creating non-proto type pb.Listing to manage version-specific marshal/unmarshal of JSON contracts and proper mapping of changing fields between current v4 and future v5 Listings.... Ex: pb.Listing.Price() can return a valid price regardless of source being v4 or v5.)
  • bump Listing to new schema v5
    • update pb.Listing.Metadata.Coin[Type|Divisibility] to use pb.CurrencyDefinition
    • update pb.ListingItem.Price to use pb.CurrencyValue
    • merge pb.Listing.Metadata.PricingCurrency with pb.Listing.ListingItem.Price into pb.CurrencyValue
    • in pb.Listing.BuyerOrder update pb.Order_Payment.Amount and pb.Order_Payment.Coin to use pb.CurrencyValue
      • replace ModeratorInfo.Price usage with pb.CurrencyValue
      • replace CoinType and pb.CoinDivisibility to use pb.CurrencyDefinition
    • update pb.ListingItem.Skus.Quantity to use json.Number (to support larger inventory requirements in CRYPTOLISTING listings)
    • update pb.Listing.BuyerOrder.RefundFee to use pb.CurrencyValue
    • in pb.Listing.BuyerOrder.Items collapse Quantity and Quantity64 ito json.Number
      • properly represent that < v2 schema expects Quantity and >= v3 requires Quantity64
      • >= v5 requires json.Number to support (now) larger inventory requirements for Cryptocurrency listings
    • in pb.Listing.BuyerOrder update Amount and Coin to use pb.CurrencyValue
    • update BuyerOrder schema version?
    • update pb.VendorOrderConfirmation.RequestedAmount to pb.CurrencyValue
    • update pb.VendorOrderFulfillment.Payout.PayoutFeePerByte to pb.CurrencyValue
    • update pb.Dispute.Outputs[].Value to pb.CurrencyValue
    • update pb.TransactionRecord.Value to pb.CurrencyValue
    • in pb.ShippingOption.Service update Price and AdditionalItemPrice to pb.CurrencyValue
    • update pb.Coupon.PriceDiscount to pb.CurrencyValue

*Note: I know there is redundant data with the currency type being expressed in every place where a value is represented. This was done intentionally so that knowledge of implicit relationships between fields aren't required to make sense of the data. Denormalizing data here also doesn't hurt us as we can ensure the implicit relationships are intact more easily with tests and validations.

Other Data Models

  • Notifications
    • New Order Notification
      • in ListingPrice merge Amount, CurrencyCode, and CoinDivisibility into pb.CurrencyValue
  • Profile
    • ModeratorInfo needs to be migrated to use the new proto schema
      • does prior ModeratorInfo schema needs to be handled for other clients when selecting a Moderator who hasn't migrated?

SQLite data

  • cases
    • migrate buyerOutputs and vendorOutputs which contain serialized sets of values: [{"hash":"85499ec9b053772fc9ce95b695de1aa1fe83ea7daf44cda52ab27aac13a679d6","value":386091}]
  • inventory
    • migrate count from type integer to text?
  • notifications
    • migrate serializedNotification to new model schema
  • purchases/sales (consider merging these into one table and add a type column to differentiate?)
    • migrate total from type integer to text?
    • migrate transactions which contain serialized sets of values: [{"Txid":"85499ec9b053772fc9ce95b695de1aa1fe83ea7daf44cda52ab27aac13a679d6","Index":0,"Value":386091,"Address":"prv2kgx6m8ynestdmvhlndm237hhqgr4yce9q0a090","Spent":true,"Timestamp":"2019-01-23T22:36:37.141428851-05:00"},{"Txid":"dde5a08c860faf7bd1b56d1528640fdbefd659ef798cfddb33e77b8b187fec4b","Index":0,"Value":-386091,"Address":"prv2kgx6m8ynestdmvhlndm237hhqgr4yce9q0a090","Spent":true,"Timestamp":"2019-01-23T22:50:58.659452463-05:00"},{"Txid":"dde5a08c860faf7bd1b56d1528640fdbefd659ef798cfddb33e77b8b187fec4b","Index":0,"Value":-386091,"Address":"prv2kgx6m8ynestdmvhlndm237hhqgr4yce9q0a090","Spent":false,"Timestamp":"2019-01-24T04:07:23.949652851Z"}]
  • stxos/txns/utxos (consider merging these into one table with a type differentiator?)
    • migrate value from integer to text?

API changes

The JSON representation of the pb.CurrencyValue type above should be used in place of int usages for amount. The typical transformation will look similar to the following other indicated otherwise:

A value of 0.12345678 BTC might be represented like this before:

{
    "data": "value",
    "amount": 12345678, // this key is inconsistent across calls
    "coin": "BTC", // this key is inconsistent across calls
    "otherdata": "othervalue"
}

This value would be represented in scientific notification using value for all significant digits and magnitude for the exponential. Thus 0.12345678 BTC, or 12345678e-8 BTC would be represented as:

{
    "data": "value",
    "amount": { // this key would match the replaced key in the BEFORE case
        "value": "12345678",
        "magnitude": -8,
        "currency": {
            "code": "BTC", // this key would always be `code`
            "divisibility": 8
        }
    },
    "otherdata": "othervalue"
}
  • POST /ob/purchase (POSTPurchase) response

wallet-interface changes

Note: Some objects (like pb.CurrencyValue and pb.CurrencyDefinition) will need to be defined within the interface's package. Recommend that ob-go abstracts the wallet-interface's version of these type with it's own to cast into as the value returns from a wallet-interface call. wallet.Type indicates a defined Type local to the package wallet.

  • create wallet.CurrencyValue and wallet.CurrencyDefinition which mirrors the data of the above protobufs
  • move CurrencyCode() string response into broader CurrencyDefinition() wallet.CurrencyDefinition
  • update IsDust(amount int64) bool to IsDust(amount wallet.CurrencyValue) bool
  • update Balance() (unconfirmed, confirmed int64) to Balance() (unconfirmed, confirmed wallet.CurrencyValue)
  • update ChainTip() (uint32, chainhash.Hash) to ChainTip() (uint64, chainhash.Hash) - Note: won't overflow until the year 4743 assuming 20-second ETH blocks which is our 'fastest' supported coin today, but would support even faster blocks in the future. None of the above scope considers a change of the ChainTip representation and must be scoped in addition to this.
    • update GetConfirmations(txid chainhash.Hash) (confirms, atHeight uint32, err error) to have same size uint as ChainTip
    • update TransactionCallback.Height to match type
  • update GetFeePerByte(feeLevel FeeLevel) uint64 to GetFeePerByte(feeLevel FeeLevel) wallet.CurrencyValue
  • update Spend(amount int64, addr btc.Address, feeLevel FeeLevel, referenceID string) (*chainhash.Hash, error) to use wallet.CurrencyValue
  • update EstimateFee([]TransactionInput, []TransactionOutput, feePerByte uint64) uint64 to EstimateFee([]TransactionInput, []TransactionOutput, feePerByte wallet.CurrencyValue) wallet.CurrencyValue
  • update EstimateSpendFee(amount int64, feeLevel FeeLevel) (uint64, error) to EstimateSpendFee(amount wallet.CurrencyValue, feeLevel FeeLevel) (wallet.CurrencyValue, error)
  • update CreateMultisigSignature(ins []TransactionInput, outs []TransactionOutput, key *hd.ExtendedKey, redeemScript []byte, feePerByte uint64) ([]Signature, error) to use wallet.CurrencyValue instead of uint64 on feePerByte
  • update Multisign(ins []TransactionInput, outs []TransactionOutput, sigs1 []Signature, sigs2 []Signature, redeemScript []byte, feePerByte uint64, broadcast bool) ([]byte, error) to use wallet.CurrencyValue instead of uint64 on feePerByte
  • change TransactionCallback
    • update Value to use wallet.CurrencyValue
  • update TransactionOutput.Value to use wallet.CurrencyValue
  • update TransactionRecord.Value to use wallet.CurrencyValue

wallet-interface dependencies

  • update each wallet implementation to support new wallet-interface changes
    • BTC
    • BCH
    • LTC
    • ZEC
    • ETH

auxiliary system dependencies

  • update inventory to support ETH precision (whenever we support ETH/ERC-20 cryptolisting trades) (scope?)
  • update search crawler/index (scope?)

placer14 avatar Jan 24 '19 22:01 placer14

Thanks for starting this @placer14!

Washington and I have been talking through this scope and this is what we want to recommend:

We're in favor of a lowest LoE change that get us what we need with minimal changes to the protobuf schema and subsequently the codebase itself. Rather than the schema you proposed, we'd like to go for a straightforward change to price amounts from uint64 (for current v4) to string (for future v5). The divisibility of each coin should be configured at the wallet interface level.

The guiding principle is : Use big.Int for computation, use string for storage/representation.

Wallet interface changes

Add Divisibility to wallet-interface#Coin struct in datastore.go

type Coin interface {
	String() string
	CurrencyCode() string
	Divisibility() int
}

func (c *CoinType) Divisibility() int {
	switch *c {
	case Ethereum:
		return 18
	case TestnetEthereum:
		return 18
	default:
		return 8
	}
}

Change Utxo#Value to be string in place of current int64. Change Txn#Value to be string in place of current int64. Change TransactionCallback#Value to be string in place of current int64. Change TransactionOutput#Value to be string in place of current int64. Change TransactionInput#Value to be string in place of current int64. Change TransactionRecord#Value to be string in place of current int64.

Current

  // EstimateSpendFee should return the anticipated fee to transfer a given amount of coins
	// out of the wallet at the provided fee level. Typically this involves building a
	// transaction with enough inputs to cover the request amount and calculating the size
	// of the transaction. It is OK, if a transaction comes in after this function is called
	// that changes the estimated fee as it's only intended to be an estimate.
	//
	// All amounts should be in the coin's base unit (for example: satoshis).
  EstimateSpendFee(amount int64, feeLevel FeeLevel) (uint64, error)

Changes to

  EstimateSpendFee(amount big.Int, feeLevel FeeLevel) (big.Int, error)

Current

  // EstimateFee should return the estimate fee that will be required to make a transaction
	// spending from the given inputs to the given outputs. FeePerByte is denominated in
	// the coin's base unit (for example: satoshis).
	EstimateFee(ins []TransactionInput, outs []TransactionOutput, feePerByte uint64) uint64

Changes to

  EstimateFee(ins []TransactionInput, outs []TransactionOutput, feePerByte big.Int) big.Int

Current

  // Spend transfers the given amount of coins (in the coin's base unit. For example: in
	// satoshis) to the given address using the provided fee level. Openbazaar-go calls
	// this method in two places. 1) When the user requests a normal transfer from their
	// wallet to another address. 2) When clicking 'pay from internal wallet' to fund
	// an order the user just placed.
	// It also includes a referenceID which basically refers to the order the spend will affect
	Spend(amount int64, addr btc.Address, feeLevel FeeLevel, referenceID string) (*chainhash.Hash, error)

Changes to

  Spend(amount big.Int, addr btc.Address, feeLevel FeeLevel, referenceID string) (*chainhash.Hash, error)

Current

  // Balance returns the confirmed and unconfirmed aggregate balance for the wallet.
	// For utxo based wallets, if a spend of confirmed coins is made, the resulting "change"
	// should be also counted as confirmed even if the spending transaction is unconfirmed.
	// The reason for this that if the spend never confirms, no coins will be lost to the wallet.
	//
	// The returned balances should be in the coin's base unit (for example: satoshis)
	Balance() (confirmed, unconfirmed int64)

Changes to

  Balance() (confirmed, unconfirmed big.Int)

Current

  // IsDust returns whether the amount passed in is considered dust by network. This
	// method is called when building payout transactions from the multisig to the various
	// participants. If the amount that is supposed to be sent to a given party is below
	// the dust threshold, openbazaar-go will not pay that party to avoid building a transaction
	// that never confirms.
	IsDust(amount int64) bool

Changes to

  IsDust(amount big.Int) bool

Current

  // GetFeePerByte returns the current fee per byte for the given fee level. There
	// are three fee levels ― priority, normal, and economic.
	//
	//The returned value should be in the coin's base unit (for example: satoshis).
	GetFeePerByte(feeLevel FeeLevel) uint64

Changes to

  GetFeePerByte(feeLevel FeeLevel) big.Int

Protobuf Changes

In moderator.proto, this L26

  message Price {
        string currencyCode = 1;
        uint64 amount       = 2; // Bitcoins must be in satoshi
    }

changes to

  message Price {
        string currencyCode = 1;
        string amount       = 2; // amount must be in satoshi/wei
    }

In contracts.proto, this L61

  message Item {
        string title               = 1;
        string description         = 2;
        string processingTime      = 3;
        uint64 price               = 4;
        ...

changes to

  message Item {
        string title               = 1;
        string description         = 2;
        string processingTime      = 3;
        string price               = 4;
        ...

this L86

  message Sku {
            repeated uint32 variantCombo = 1;
            string productID             = 2;
            int64 surcharge              = 3;
            int64 quantity               = 4; // Not saved with listing
        }

changes to

  message Sku {
            repeated uint32 variantCombo  = 1;
            string productID              = 2;
            string surcharge              = 3;
            int64 quantity                = 4; // Not saved with listing
        }

this L103

  message ShippingOption {
        string name                         = 1;
        ShippingType type                   = 2;
        repeated CountryCode regions        = 3;
        repeated Service services           = 5;

        enum ShippingType {
            LOCAL_PICKUP = 0;
            FIXED_PRICE  = 1;
        }

        message Service {
            string name                = 1;
            uint64 price               = 2;
            string estimatedDelivery   = 3;
            uint64 additionalItemPrice = 4;
        }
    }

changes to

  message ShippingOption {
        string name                         = 1;
        ShippingType type                   = 2;
        repeated CountryCode regions        = 3;
        repeated Service services           = 5;

        enum ShippingType {
            LOCAL_PICKUP = 0;
            FIXED_PRICE  = 1;
        }

        message Service {
            string name                = 1;
            string price               = 2;
            string estimatedDelivery   = 3;
            string additionalItemPrice = 4;
        }
    }

this L129

  message Coupon {
        string title = 1;
        oneof code {
            string hash         = 2;
            string discountCode = 3;
        }
        oneof discount {
            float percentDiscount = 5;
            uint64 priceDiscount  = 6;
        }
    }

changes to

  message Coupon {
        string title = 1;
        oneof code {
            string hash         = 2;
            string discountCode = 3;
        }
        oneof discount {
            float percentDiscount = 5;
            string priceDiscount  = 6;
        }
    }

this L142

  message Order {
    string refundAddress                 = 1;
    uint64 refundFee                     = 2;
    ...

changes to

  message Order {
    string refundAddress                 = 1;
    string refundFee                     = 2;
    ...

this L185

  message Payment {
        Method method       = 1;
        string moderator    = 2;
        uint64 amount       = 3; // Satoshis
        ...

changes to

  message Payment {
        Method method       = 1;
        string moderator    = 2;
        string amount       = 3; // Satoshis or Wei
        ...

this L203

  message OrderConfirmation {
    string orderID                            = 1;
    google.protobuf.Timestamp timestamp       = 2;
    // Direct payments only
    string paymentAddress                     = 3;
    uint64 requestedAmount                    = 4;

    repeated RatingSignature ratingSignatures = 5;
  }

changes to

  message OrderConfirmation {
    string orderID                            = 1;
    google.protobuf.Timestamp timestamp       = 2;
    // Direct payments only
    string paymentAddress                     = 3;
    string requestedAmount                    = 4;

    repeated RatingSignature ratingSignatures = 5;
  }

this L282

  message Payout {
        repeated BitcoinSignature sigs = 1;
        string payoutAddress           = 2;
        uint64 payoutFeePerByte        = 3;
    }

changes to

  message Payout {
        repeated BitcoinSignature sigs = 1;
        string payoutAddress           = 2;
        string payoutFeePerByte        = 3;
    }

this L329

  message DisputeResolution {
    google.protobuf.Timestamp timestamp = 1;
    string orderId                      = 2;
    string proposedBy                   = 3;
    string resolution                   = 4;
    Payout payout                       = 5;
    repeated bytes moderatorRatingSigs  = 6; // Used in ratings

    message Payout {
            repeated BitcoinSignature sigs = 1;
            repeated Outpoint inputs       = 2;
            Output buyerOutput             = 3;
            Output vendorOutput            = 4;
            Output moderatorOutput         = 5;

            message Output {
              oneof scriptOrAddress {
                string script  = 1;
                string address = 3;
              }
              uint64 amount  = 2;
            }
    }
  }

changes to

  message DisputeResolution {
    google.protobuf.Timestamp timestamp = 1;
    string orderId                      = 2;
    string proposedBy                   = 3;
    string resolution                   = 4;
    Payout payout                       = 5;
    repeated bytes moderatorRatingSigs  = 6; // Used in ratings

    message Payout {
            repeated BitcoinSignature sigs = 1;
            repeated Outpoint inputs       = 2;
            Output buyerOutput             = 3;
            Output vendorOutput            = 4;
            Output moderatorOutput         = 5;

            message Output {
              oneof scriptOrAddress {
                string script  = 1;
                string address = 3;
              }
              string amount  = 2;
            }
    }
  }

this L365

  message Refund {
    string orderID                      = 1;
    google.protobuf.Timestamp timestamp = 2;
    repeated BitcoinSignature sigs      = 3;
    TransactionInfo refundTransaction   = 4;
    string memo                         = 5;

    message TransactionInfo {
        string txid  = 1;
        uint64 value = 2;
    }
  }

changes to

  message Refund {
    string orderID                      = 1;
    google.protobuf.Timestamp timestamp = 2;
    repeated BitcoinSignature sigs      = 3;
    TransactionInfo refundTransaction   = 4;
    string memo                         = 5;

    message TransactionInfo {
        string txid  = 1;
        string value = 2;
    }
  }

In api.proto this L45

  message TransactionRecord {
    string txid                         = 1;
    int64 value                         = 2;
    uint32 confirmations                = 3;
    uint32 height                       = 4;
    google.protobuf.Timestamp timestamp = 5;
  }

changes to

  message TransactionRecord {
    string txid                         = 1;
    string value                        = 2;
    uint32 confirmations                = 3;
    uint32 height                       = 4;
    google.protobuf.Timestamp timestamp = 5;
  }

API Changes

int => string type conversions for amounts will need to be changed in several locations. Beyond that, no special additions are required to the existing responses.

The conversion from string to big.Int will happen in the core package. Functions such as core/order.goL76 Purchase should return string for amount.

Repo/Sqlite DB changes

Migrate exisiting amount fields from int to string.

Sqlite does not support modifying column type as described here: https://www.sqlite.org/lang_altertable.html

The solution given is to create a temp table with the required column changes, start a transaction, insert data from old table to temp table, drop the old table, finally rename the temp table to the old table name.

The proposed wallet.Currency seems to be an overkill for the following reasons:

  1. The coin does not change for a contract
  2. There are no intermediate values being stored for a coin. All the amounts are stored in the finest granularity.
  3. Like the coin, there is no change in the divisibility of a coin in the contract.
  4. Till the time that Openbazaar does intend to support multi coin contract with varying divisibility, the proposed solution adds a lot more complexity for little mileage.

Also, shopspring/decimal is not being considered because there is no float value storage or processing in ordering.

Overall we believe this is a much simpler and direct path forward to supporting ETH-level precision that minimises schema and codebase changes.

amangale avatar Jan 31 '19 11:01 amangale

After discussing the suggestion in private, @amangale is on board with the plan as originally described above. For historical purposes, my observations are outlined below:

Some thoughts after reviewing Ashwin's proposal:

(TL;DR)

  • Ashwin’s proposal doesn’t reduce scope much compared to the original plan of inlining divisibility within the contract. (See 3.)
    • The main delta appears to be rejecting the custom type in favor of big.Int and implicitly defined data. The difference between supporting big.Int and custom types:
    • Cons: Minor risk in implementing MarshalJSON and accessor functions. More complex structure.
    • Pros: A type which can answer IsValid() no matter what version it is. Access ambiguously defined data in the wire based on version without the consumer knowing how. Cleans up existing debt.
  • There are aspects of Ashwin's approach which does increase risk/tech debt. (See 1, 2)
    • Does not consider transient or legacy data support.
    • Requires a piecemeal logic in specific areas than something more holistic to the app's ecosystem
  • If we do the wallet-only fix first as we have suggested doing, Ashwin’s proposal causes bigger tech debt than just inlining the currency divisibility into the contract. (See 1.)

This proposal does not address in-flight contracts and our ability to support them. Providing a proper abstraction (repo.RicardianContract) which is version-aware and can be a place to hang validation/business-logic/etc is going to be needed sooner or later. If we avoid this abstraction, we're simply sprinkling copies of this logic throughout the codebase which we'll need to later reconcile (more debt).

I would personally prefer to see:

(repo.RicardianContract).contract.BuyerPayment().Definition.Divisibility or (repo.RicardianContract).contract.BuyerPayment().Divisibility

than having this sprinkled across our codebase:

if contract.Metadata.Version > 5 {
  divisibility = wallet.GetWalletForCurrencyCode(coin).CoinDivisibility()
} else {
  divisibility = MagicDivisibilityIntThatIsNotLinkedToContractData
}

Considering this will be a two-step approach in which the first step allows ETH contracts containing "satoshi-wei" to be used on the network, there is no explicit divisibility data associated with the amounts to know the correct magnitude when we eventually implement full precision support in step two. Assuming we did this, in step two we'd need a new field with explicit divisibility or a different name with yet more logic in the server. It would seem that adding divisibility in the first place would be prudent.

And if we're already going down the path of including this explicit data, good engineering would be to properly represent this data instead of injecting string arguments throughout our interfaces and requiring the engineer to digest and understand the code body in order to understand what the "string" is. Beyond semantics, it's poor because there's no place to hang your behavior specific to CurrencyValues without writing and creating helpers all over your code. This is not only debt, but it's high-interest.

Responses for your arguments of overkill for a wallet.CurrencyValue and wallet.CurrencyDefinition type:

Till the time that Openbazaar does intend to support multi coin contract with varying divisibility, the proposed solution adds a lot more complexity for little mileage.

I'll start here as it's most important. I'm not sure what complexity we're avoiding in your proposal. The only main delta that I can see (forgive me if I'm wrong here as it was hard to compare them) is that you rejected the custom type usage in favor of a string representation throughout the app. All of the same data points are being touched to change the type to a more basic representation (to use big.Int with implicit data instead of a descriptive type). Objectively, the only added complexity I see is writing MarshalJSON for the Currency* types and accessor methods.

The coin does not change for a contract Like the coin, there is no change in the divisibility of a coin in the contract.

If you're suggesting that one expression of the coin metadata is enough for an entire contract, I disagree. When we describe a Cryptocurrency Listing, the coin can be represented as a product of purchase as well as a means of payment. This necessitates independant representation of these values. If we're expressing the same data in both places, a consistant representation of the data would be preferred.

Here's an example listing:

{
	"vendorListings": [{
		"slug": "bch-eth",
		"vendorID": {
			"peerID": "QmbweQKGWNqjoPqBjePVqnTCrFsK62oWYeVe8WeqXwwyS8",
			"handle": "",
			"pubkeys": {
				"identity": "CAESIM8hU+iL8iGmlzAIMI1OGqRMWXRwNGcbW8nXOZh0eurL",
				"bitcoin": "A/n06c7eeuBo9/mWLgXQGacFI7+gnAXBlLkquCfv0srk"
			},
			"bitcoinSig": "MEUCIQCftGYX8xF+oGEZ+scMqvEasqLvh0JQ92KO81ZNg3bfMAIgIQNOpyW7eIfR50YefdF/aj9jofL491PRgFvdYHUBT24="
		},
		"metadata": {
			"version": 4,
			"contractType": "CRYPTOCURRENCY",
			"format": "MARKET_PRICE",
			"expiry": "2037-12-31T05:00:00.000Z",
			"acceptedCurrencies": ["BCH"],
			"pricingCurrency": "",
			"language": "",
			"escrowTimeoutHours": 1,
			"coinType": "ETH",
=>			"coinDivisibility": 100000000,   // payment or inventory?
			"priceModifier": 5
		},
		"item": {
			"title": "BCH-ETH",
			"description": "Cheap cryptos!",
			"processingTime": "",
			"price": 0,
			"nsfw": false,
			"tags": [],
			"images": [{
				"filename": "786a.gif",
				"original": "zb2rhn3zyAYj17JenXKGpJPBTRn68g8PJ6ZMXWFvRr7BTLLib",
				"large": "zb2rhh2BLagyb4yG8kSCTtrChHS45JAE2rLp92Zi3RwdZKK1B",
				"medium": "zb2rhnppMGkZYp6Zg7Qf2irDH9z1ZM5jc2VcAXfy6mEnifoEy",
				"small": "zb2rhfMZFaaWZxZGvkqAPCMUbmdxNHhAaCby5XCkRrV13bew8",
				"tiny": "zb2rhfN4RQyNP6eZszvEBfwBRMZxaysoqF72MYWPKoofV5AQr"
			}],
			"categories": [],
			"grams": 0,
			"condition": "",
			"options": [],
			"skus": [{
				"productID": "",
				"surcharge": 0,
				"quantity": 0
			}]
		},
		"shippingOptions": [],
		"coupons": [],
		"moderators": [],
		"termsAndConditions": "",
		"refundPolicy": ""
	}],
	"buyerOrder": {
		"refundAddress": "qreeeu24nq4f3pc5wel4ucwaxpg3ug35lg5t33566e",
		"refundFee": 0,
		"shipping": {
			"shipTo": "",
			"address": "",
			"city": "",
			"state": "",
			"postalCode": "",
			"country": "NA",
			"addressNotes": ""
		},
		"buyerID": {
			"peerID": "Qmb8CvXLDChhU5YBApLKbs8ixfWh9X6dmPHbp6B5tjFqB1",
			"handle": "",
			"pubkeys": {
				"identity": "CAESIHh9uLUPanFQVVIFkYsypxCezy/JYAx/pqu2cjUYIDZo",
				"bitcoin": "A0/wPzw14M6i/l+INxQY3JdvVOkV2lLNev+TxVjGBxS6"
			},
			"bitcoinSig": "MEQCIEx4s0UxxMK44+6KZhJAyQE64DtH9IJg0u38bkXbvfJxAiAGy6iVzHJwaMB3Njaq73+RrQ91VQnhqyc26o4GGi9XSg=="
		},
		"timestamp": "2019-01-31T21:12:20.170965750Z",
		"items": [{
			"listingHash": "zb2rhgg9mdHRub6DaBKBTu8Akg9UYYdz4azpBpdVKKNhTqMFB",
			"quantity": 0,
=>  		"quantity64": 1,        // 1 what?
=>			"preciseQuantity": "1", // future field inclusion makes these quantity fields ambiguous
			"memo": "",
			"paymentAddress": "Addresss"
		}],
		"payment": {
			"method": "ADDRESS_REQUEST",
			"moderator": "",
			"amount": 0,
			"chaincode": "",
			"address": "",
			"redeemScript": "",
			"coin": "BCH"
		},
		"ratingKeys": ["Ar1GMyRSUfS+rVZ7k8SOktFPISgU1wBu3jJkQ2Sci2eP"],
		"alternateContactInfo": "",
		"version": 2
	},
	"vendorOrderConfirmation": {
		"orderID": "QmVMf7iXQxvgbo1r6JoiT22Zyz334mV6xoQmN5bLfB5LVY",
		"timestamp": "2019-01-31T21:12:21.258215870Z",
		"paymentAddress": "qpf0sv9d5q9rurg40z3es9270j7hktv9fsncfegvlk",
		"requestedAmount": 0
	},
	"signatures": [{
		"section": "LISTING",
		"signatureBytes": "ga81iV5H6UnFQ1d8HVBjLapNCvLkgFSBd2ziNMQ2Bez7DHXcrL5+qynPeT2TbqPIUvqySAUQleNDSRTEEWRWDA=="
	}, {
		"section": "ORDER",
		"signatureBytes": "FzXD5GmxHgdzSL9FkgHeSs4BLApG3t8dOTulJEwCKEdP9tOWIZU8ZWLf5veiuw/Rn9OtSVgBAzz6pHeI8K4pCw=="
	}, {
		"section": "ORDER_CONFIRMATION",
		"signatureBytes": "m/mOeypWiz9epxwrmcENJjSVQF6ITpaGnvCCtNzF7k/YO8eTSMKr45ZqitAznhZsQbKl4O99mgAakqlODGlzAw=="
	}]
}

If you're suggesting that the coin will never change over time (as in from the time that contract is created), I disagree for two reasons. Contracts have technically infinite lifetime as new orders may be executed at any time in the future. While there's precedent for us to void old versions of contracts with new versions of the server, this has been in extreme scenarios and is avoided at all costs. (And is really just a cost of not paying the debt outlined in point (1) above.) More importantly, is the case I outlined in point (2) above. If the wallet is accepting satoshi-wei now and later actual-wei and there's no way to differentiate that in the contract, the contract suddenly becomes a lie (for the value spent) because the relationship between the contract and the server API are non-existent and there's no way to resolve that.

Not to mention the cases where a coin would hardfork to create a larger supply (particularly in the case of utility coins).

There are no intermediate values being stored for a coin. All the amounts are stored in the finest granularity.

No argument from me there. I agree less complexity is preferred over more complexity. But (2) describes why we need a more complex representation. This is an obvious technical case we need to support if we decided to take a two-step approach.

Will prepare planning out the above epic into executable steps for planning and discussion.

placer14 avatar Feb 01 '19 14:02 placer14

Additionally, we briefly discussed the plan of attack for this large scope w/ @drwasho and @amangale. The idea was raised to begin in the ETH wallet implementation as it is the highest risk and could be nice to have the chance to abort the plan if something comes up. I agree with this and with this thought in mind, here's my thinking about how to execute the beginning:

  1. wallet interfaces for CurrencyValue and CurrencyDefinition. Prefer these types to be separate so a wallet-interface could deliver func Definition() *wallet.CurrencyDefinition in place of func CurrencyCode() string
  2. Support serializing new data into the datastore (for ETH only right now, keeping in mind how this must work for all wallet implementations)
  3. datastore migrations which support the wallet interface (stxos/txns/utxos contain balance whose type needs to be updated)
  4. Update wallet-interface to use new data types

Let's plan to talk through this scope today.

placer14 avatar Feb 01 '19 15:02 placer14