saturn-network / saturn.js

Javascript client for Saturn API
9 stars 8 forks source link

Query order balance via smart contract bypassing API #1

Open saturn-network opened 5 years ago

saturn-network commented 5 years ago

Using Saturn API simplifies a lot of the tricky bigint math that is essential in making Saturn Network exchange work. There are certain places where living without API is almost impossible, i.e. serving the order book for a token in under <100ms - you'd have to reimplement the API yourself. The API simply acts as a high-performance low-latency cache, and blockchain acts as the slow & secure source of truth for it.

Querying for the blockchain directly for the order balance, however, right before executing the order makes perfect sense. A good first contribution would be to modify this function to stop relying on the API and query the blockchain directly.

https://github.com/saturn-network/saturn.js/blob/f06ac6373d9af848cd0e71a175556f65fa002105/src/exchange/exchange.ts#L217-L223

Some helpful notes to get you started:

  1. Review package.json of saturn.js and look at the scripts section. Hint: npm run build does something useful.
  2. Learn to use yarn link or npm link to test drive your local copy of saturn.js with your scripts.
  3. Some code snippets that you might find useful
    let decimals = await token.decimals()
    let rawOrderBalance = await exchange.remainingAmount(order.order_id)
    let parsedOrderBalance = new Fraction(rawOrderBalance.toString()).div(new Fraction(10).pow(Number(decimals.toString())))
saturn-network commented 5 years ago

Bounty: 69,000 SATURN tokens of your choosing courtesy of Neuron.

PseudoDeterminist commented 5 years ago

The bad balance number seems to be coming from the ABI. Note the balance in the ABI query is:

"balance":"828.18209999034782608696"

When I try to sell into that using pricewatch-bot I get

You attempted to trade more tokens (828.1821) than are available in the order (828.18209999034782608696) for order_tx 0xcc4923e34e9ab99cf83fc88a1c98cf114d45760d9a0e8eca13c7206fae023d86.

I'm going to read up on how saturn.js can query the balance. I'll get it. No questions - just progress.

curl https://ticker.saturn.network/api/v2/orders/by_tx/ETC/0xcc4923e34e9ab99cf83fc88a1c98cf114d45760d9a0e8eca13c7206fae023d86.json {"created_at":1557095984,"transaction":"0xcc4923e34e9ab99cf83fc88a1c98cf114d45760d9a0e8eca13c7206fae023d86","blockchain":"ETC","order_id":"3162","blocknumber":"7980452","contract":"0x3ec00ee8a4fbe81e7eea328029ce490654e8e11a","owner":"0x5860b14f9ac5c4ddaff8775a6c84b49a6c28dd07","active":true,"buytoken":{"address":"0xac55641cbb734bdf6510d1bbd62e240c2409040f","decimals":4,"blockchain":"ETC","name":"Saturn Classic DAO Token","symbol":"SATURN"},"selltoken":{"address":"0x0000000000000000000000000000000000000000","decimals":18,"blockchain":"ETC","name":"Classic Ether","symbol":"ETC"},"balance":"828.18209999034782608696","price":"0.000046","type":"BUY"}%

PseudoDeterminist commented 5 years ago

Confirmed (as I'm sure you have) that order is not tradeable on saturn.network or by bot. Also, attempted cheesy hack of saturn.js by modifying verifyCapacity as follows:

  if (amount > Number(order.balance)) {
    amount = Number(order.balance)
  }

instead of throwing an error. This (I think) attempts to do the transaction as if the trader amount was containing the 9999's. In any case it stops the error condition so the trade can be attempted.

Then threw pricewatch-bot at it and got

Attempting to sell 828.1821 tokens tx: 0xa1ea669529cd1abf76afe3f6ea7f677f8a0e4ca9e0fdc84054e7bbdea17832d5 Transaction 0xa1ea669529cd1abf76afe3f6ea7f677f8a0e4ca9e0fdc84054e7bbdea17832d5 on ETC failed.

That just updates progress report.

Next I'm still going to (do, not try!) what Neuron asked me to do, which is use saturn.js to query the balance, see if I can get rid of those ugly 9999's one way or another.

No questions.

PseudoDeterminist commented 5 years ago

Okay for completeness:

I cheesy-modified verifyCapacity in saturn.js as follows:

if (amount > Number(order.balance)) { order.balance = amount }

instead of throwing an error.

This (I think) attempts to sell into that order as if the order balance were in fact 828.1821. Although I'm not certain about that, as this was a noob cheesy hack.

In any case the transaction fails again, with a bad instruction error showing on blockscout:

https://blockscout.com/etc/mainnet/tx/0x6b915cbf07ec1a3b49fa1db3a9461a28f99f118e625779a1d9dfeb118cc031c3/internal_transactions

None of these three things are what Neuron asked me to do, and all of them are cheesy hacks, but it was my first day, and I wanted to know what these would do.

Still working on using saturn.js and the given code snippets to query the balance that way.

No questions.

PseudoDeterminist commented 5 years ago

Okay this is where I've been spinning my wheels for days: I added the following code (between // lines) to newERC223Trade:

  private async newERC223Trade(tokenAddress: string, amount: number, order: Order) : Promise<string> {
    console.log('--> newERC223Trade')
    let token = new Contract(tokenAddress, erc223, this.wallet)
//
    let decimals = await token.decimals()
    let exchange = new Contract(order.contract, this.exchangeAbi as any, this.wallet)
    let rawOrderBalance = await exchange.remainingAmount(order.order_id)
    let parsedOrderBalance = new Fraction(rawOrderBalance.toString()).div(new fraction(10).pow(Number(decimals.toString())))
    console.log(Number(rawOrderBalance), Number(parsedOrderBalance))
//

Basically all I want to do is query that rawOrderBalance and log it on the console!

What I get I cannot make sense of. Logged as objects, I get

rawOrderBalance: BigNumber { _hex: '0x056a1b3773be983c' }
parsedOrderBalance:  { s: 1, n: 24384640168749444, d: 625 }

Logged as a string or number, I get

rawOrderBalance: 474999998400000000
parsedOrderBalance: 1956513536

And this is for an order with no rounding problem, balance of: 9895.8333

The sale goes through in pricewatch-bot just fine for this order. I can query order.balance just fine but I think that comes from the SaturnAPI web3 interface.

rawOrderBalance is stumping me.

PseudoDeterminist commented 5 years ago

One thing I've learned from this adventure is funds are definitely safu. I see that pricewatch-bot and saturn.network interface trade correctly down to the smallest Saturn unit. Some bug in the beta release of market-maker-bot is allowing it to post weird problematic balances onto the exchange, but I imagine that doesn't worry you guys as to how to fix! I imagine that can also be prevented in SaturnAPI pretty easily, both as an error protection measure and to stop a potential attack on the exchange.

I already respected you guys highly, but having spent a few days trying to dig into your code, the respect has only gone way up. I couldn't have picked a better project to try my hand at.

I'd like to keep doing this. I know I can't waste your time asking too many questions. I'll keep studying anyway.

saturn-network commented 5 years ago

Notice that the exchange smart contract returns balance in selltokens. That is, the balance is denominated in collateral. If it is a buy order, then the returned balance is denominated in ether. If it is a sell order then the returned balance is denominated in tokens.

Saturn API always returns the balance of the order converted to tokens.

And this is for an order with no rounding problem, balance of: 9895.8333

Not surprised - newERC223Trade is being called against a buy order, so you get balance denomicated in ether and then divide it by token decimals.

Highly suggest you work on modifying verifyCapacity function that gets called in the right place as opposed to changing newERC223Trade that needs no alterations.

PseudoDeterminist commented 5 years ago

Unbelievable that my unstoppable and brilliant mind didn't think to check if the balance might be in ethers. Thanks for rescuing me...

First things first: Both of the orders for .0001 Saturn Classic belong to my wallets. I'll cancel them if you want them cleared. Or you can sell into them if you want, or whatever, using your tricks. I'd kinda like to do that myself, but I get that I'm taking a long time learning.

Second, (code shown at bottom of post) I queried the rawOrderBalance and got:

rawOrderBalance 4699991100 parsedOrderBalance 469999.11
amount: 0.0001

amount is how much saturn.js thinks we are trying to sell in Saturn tokens. it's correct, of course. but it doesn't agree with the 'order' object in saturn.js (queried from verifyCapacity):

{ created_at: 1557207769,
  transaction:
   '0x16687b936e556254a5a8cf99163fdd8339532d895aac6c85c69e8a52264b7731',
  blockchain: 'ETC',
  order_id: '3184',
  blocknumber: '7988220',
  contract: '0x3ec00ee8a4fbe81e7eea328029ce490654e8e11a',
  owner: '0xbad82c4735546a1f8a032ffd000cc46a9b8fa31a',
  active: true,
  buytoken:
   { address: '0xac55641cbb734bdf6510d1bbd62e240c2409040f',
     decimals: 4,
     blockchain: 'ETC',
     name: 'Saturn Classic DAO Token',
     symbol: 'SATURN' },
  selltoken:
   { address: '0x0000000000000000000000000000000000000000',
     decimals: 18,
     blockchain: 'ETC',
     name: 'Classic Ether',
     symbol: 'ETC' },
  balance: '0.00009999981063829787',
  price: '0.000047',
  type: 'BUY' }

what does agree with order.balance is rawOrderBalance (in ethers) times the sell price. But that doesn't agree with 'amount', it's off by tiny bit.

rawOrderBalance tells me that to sell into this order, (I think) I need to send exactly this much ethers: 0.0000000046999911, to the correct contract call. I don't know how do do that yet. But I think that might work!

One thing I tried was to just approve the .0001 amount in verifyCapacity (remove the if condition!) -->transaction is sent, but fails with a bad instruction. Note that the above amount (.0001 Saturn) times the price (0.000047) gives: 0.0000000047 ethers. So it will try to send tiny bit more than rawOrderBalance. I think this causes the fail?

I realize the task here is to just query the order balance directly. I've done that. But I really have no code to pull request yet... just used modified verifyCapacity with added code between // lines. When I send a pull request, I'm hoping it will actually do something with the query? Like even succeed in selling an order like this one?

My code modification in verifyCapacity is between the // lines:

private async verifyCapacity(amount: number, order: Order) {
      console.log('--> verifyCapacity')
//
    console.log(order)
      let tokenAddress = order.buytoken.address
      let token = new Contract(tokenAddress, erc223 as FunctionFragment[], this.wallet)
      let decimals = await token.decimals()
      let exchange = new Contract(order.contract, this.exchangeAbi as any, this.wallet)
      let rawOrderBalance = await exchange.remainingAmount(order.order_id)
      let parsedOrderBalance = new Fraction(rawOrderBalance.toString()).div(new Fraction(10).pow(Number(decimals.toString())))
      console.log('rawOrderBalance', Number(rawOrderBalance), 'parsedOrderBalance', Number(parsedOrderBalance))
      console.log('amount:', amount)
//
//    if (amount > Number(order.balance)) {
//      throw new Error(`
//        You attempted to trade more tokens (${amount}) than are available in the order (${order.balance}) for order_tx ${order.tran\
saction}.
//      `.trim())
//    }
  }

I don't know how to send the right amount of ethers (yet), though I know how many to send! I don't think we can send less than 0.0001 Saturn as the 'amount' to saturn.js

But one or the other of those is what this order would need.

PseudoDeterminist commented 5 years ago

Oops I said send ethers to a Saturn buy order to clear it. That's dumb. So yeah, you'd have to send .0001 Saturn... and we tried that. Now what?

saturn-network commented 5 years ago

So it will try to send tiny bit more than rawOrderBalance. I think this causes the fail?

Yes

saturn-network commented 5 years ago

sending less than 0.0001 Saturn is impossible. Sending less Ether is definitely possible.

In terms of your contribution, cleaning up verifyCapacity is a good idea. As far as use case with the bots, changes will need to be made there. The goal of saturn.js is to simply be boring pipes that take you commands and execute them or give good error messages if you are doing something wrong.

Business logic should live elsewhere, i.e. in the bot code. After making the change in this library you might want to attempt to fixup this method so it returns correct results even with tiny amounts.

https://github.com/saturn-network/pricewatch-bot/blob/4136d35557cac59337e2ca5a87742158194d2657/pricewatch-bot.js#L89-L103

PseudoDeterminist commented 5 years ago

I left an order of 'balance' 82.387399999802125 to give my pending pull request something to chew on. Obviously you can easily clear it, but if you could leave it for a day I'll have time to get back to this tonight. Thanks!

PseudoDeterminist commented 5 years ago

Good grief I don't know how (and I'm not asking!) to divide two freaking numbers using these data structures. I know how to hang my program just by asking for wei divided by 10^etherDecimals.

If I take a long time to do a simple task, it's because every step I take involves learning a new javascript package. Great stuff! But all of it is new to me.

No questions.

edit -- I'm starting to learn the value of the functions Number() and toSuitableBigNumber() edit 2 -- I'm literally just wandering through all this code and staring at it in awe. Trying to absorb its meaning by bouncing around in it enough. Wasting time! But wow. Why am I doing this? Because I want to do a simple operation that I don't know syntax or data structure for, and go looking... usually I can find it but I do get sidetracked on the way! Right now I want to get pricewatch-bot to sell into my order even though it's not the highest buy order. So, off on a little trek to understand how all that works.

saturn-network commented 5 years ago

ethers.js has a horrible BigNumber library that the user is forced to use... toSuitableBigNumber would ideally never exist. It is a crutch that lets us connect the rest of the program with ethers.js

bignumber.js used in bots is a much friendlier library

saturn-network commented 5 years ago

Right now I want to get pricewatch-bot to sell into my order even though it's not the highest buy order.

Look no further than the README of this project

let orderTx = `0xdeadbeef` // make sure this order is created by another address, because the smart contract prevents wash trading from same address!
let order = await saturn.query.awaitOrderTx(orderTx, saturn.etc)
let tradeTx = await saturn.etc.newTrade(order.balance, orderTx)
await saturn.query.awaitTradeTx(tradeTx, saturn.etc)
PseudoDeterminist commented 5 years ago

The order I'm working on is for

82.3874 Saturn. I'll call that

823874 sam, where sam = smallest Saturn subunit.

When I query the wei balance of this order, I get

weiOrderBalance: 3295495999992085

which is already wrong, but here I go and make it worse:

  let etherOrderBalance = new Fraction(Number(weiOrderBalance)).div(new 
     Fraction(10).pow(etherDecimals))

etherOrderBalance.toString(): 0.003295495999992

Here we've lost the last three decimal places of precision (which is okay as they were wrong anyway), but I don't know how to predict what happens when we do that division operation. When does it cut off, and why? I don't really want to know. It's just making things worse.

This is how the conversions are being done all over saturn.js (and I get why).

......

Fractions themselves are exact, but more than we need. And decimal division is a mess.

But of course the price is .00004, or 4000000000 wei per sam. So the actual wei order balance should be:

price samToBuy = 4000000000 823874 = 3295496000000000 wei.

That is a big, beautiful round number that need never be divided incorrectly, throughout the order placement and all calls to the order until the order is cleared with exactly zero balance in wei and sam.

Notice that if someone sells 17.1927 Saturn into that order, the math will go like this:

171927 sam * 4000000000 wei / sam = 687708000000000 wei subtracted from the order.

......

If I begin writing my math in subunit integer form, I'll never call Fraction, or BigNumberWhatever.div, or .mul which is just as bad in the case of decimals past the decimal point. Just big integer math.

Let's do that sell order for 171927 sam:

3295496000000000 wei - 687708000000000 wei = 2607788000000000 wei,

another big beautiful round number that happens to exactly correspond to an order:

2607788000000000 wei / 4000000000 (wei / sam) = 651947 sam.

......

Note the difference between the correct wei balance for my buy order, and the queried wei balance: 3295496000000000 wei - 3295495999992085 wei = 7915 wei.

But this is no edge case! This is how saturn.js does its math in lots of places. In this case that tiny difference was enough to throw an error, but in all cases it will lead to lots of "parsing" of numbers that were screwed up unnecessarily. It will lead to leaving dust lying around, sometimes in orders that had to be ignored instead of properly closed out.

......

I think Saturn Network is headed for millions or billions of transactions in the long run. We don't want to leave dust lying around from a significant portion of those. We want to account for dust, and do something intentional with it (maybe give it to miners? I don't know if that's possible.)

But if we have dust, we should put it in a useful pile somewhere if we can. And mostly, we should not have dust!

Let's not lose our wei.

So I will submit this pull request with integer math for a starter, and see how you like it.

saturn-network commented 5 years ago

In CS numbers have precision. Number sucks for precision (hence the need for bignumber), so when you convert to fraction here

let etherOrderBalance = new Fraction(Number(weiOrderBalance)).div(new 
     Fraction(10).pow(etherDecimals))

use toString() instead. converting to Number is a measure of last resort, only for experienced users!

try

let etherOrderBalance = new Fraction(weiOrderBalance.toString()).div(new 
     Fraction(10).pow(etherDecimals))
PseudoDeterminist commented 5 years ago

So... my last post didn't age well.

I wrote:

But this is no edge case! This is how saturn.js does its math in lots of places....

So I took the time to actually prove it was no edge case... and it was an edge case.

I put up a normal order on saturn.network and ran my numbers on it while logging the prices and values. And it went exactly correctly, down to the last Saturn subunit and the last wei. Meaning, yes I learned something: how these numbers should go. But that is exactly how they are going, except in the case of the beta market-maker-bot generated order. Making this an edge case. In beta-release software...

So, I learn something and I rush off to fix the world that ain't broke. Funds are safu! Time to go learn a little more.

You wrote:

In CS numbers have precision. Number sucks for precision....

I forgot to check what Number returns... not a big integer or a BigNumber! Just a number with ordinary precision. So that ain't gonna works. ""Ehh.. sorry, Chief!" (Maxwell Smart).

I still want to try big integers... but haven't yet. Thanks for your patience and guidance.

PseudoDeterminist commented 5 years ago

BTW if you put back my two 0.0001 Saturn orders, I will cancel them properly.

PseudoDeterminist commented 5 years ago

Okay. A little more progress. Using the following code:

  let weiOrderBalance = await exchange.remainingAmount(order.order_id)
  let ethersjsOrderBalance = utils.formatEther(weiOrderBalance)
  let etherOrderBalance = new Fraction(weiOrderBalance.toString()).div(new 
      Fraction(10).pow(etherDecimals))

We get the wei order balance (it will work the same for the token balance on the exchange): weiOrderBalance.toString(): 35130199970200 An integer. Perfect.

And we get the ether order balance using two methods: (weiOrderBalance / 10^18).toString(), etherOrderBalance: 0.000035130199970

and utils.formatEther(weiOrderBalance) ethersjsOrderBalance: 0.0000351301999702

These differ by 200 wei and the one from ethers.js utils matches the wei in the contract exactly.

Not sure why Fraction toString() left a hanging zero, but it also left off 200 wei. Another example of how it throws us off. We might be able to get it to use more precision in output, but I think with infinite repeating decimals we are always gonna have issues using division and multiplication for unit conversion.

I can do my conversions with ethers utils. I think they don't do division and multiplication. They just move decimal points to "format" the numbers. Which is perfect for what I need. I don't need any Big Integer package. I can do what I need with BigNumbers as integers. All my math can be done on the correct integers, and all my formatting can be done without actually doing division or multiplication past the decimal point. Which will prevent the introduction of precision errors.

Almost there, to posting my first pull request. Anywhere, ever.

PseudoDeterminist commented 5 years ago

Everything I have said against BigNumbers is wrong.

Assumptions kill. I saw the decimal conversion errors, assumed they were in BigNumber (whatever that was) and didn't actually go check til last night.

So as you knew and I had to learn, bignumber.js is a decimal package, which turns out to means something. It can do power-of-ten conversions all day long and never introduce errors.

Oops. Sorry about that, Chief!

So where are the errors coming from? (I ask myself)

I'm going to look into maybe our Fraction conversions. For example, these lines that gave me errors yesterday:

let etherOrderBalance = new Fraction(weiOrderBalance.toString()).div(new 
  Fraction(10).pow(etherDecimals))
console.log(etherOrderBalance.toString())

Yeah I didn't check the default precision of that toString() method on Fraction, vs the toString() method on BigNumber. They might be different!

Little embarrassed here.

PseudoDeterminist commented 5 years ago

a = new Fraction(5000).div(new Fraction(10).pow(18)) { s: 1, n: 1, d: 200000000000000 } a.toString() '0.000000000000005' a.div(10).toString() '0.000000000000000'

b = BigNumber(5000).dividedBy(BigNumber(10).pow(18)) 5e-15 b.toString() '5e-15' b.toFixed() '0.000000000000005' b.dividedBy(10).toFixed() '0.0000000000000005'

So. I would love to see that maker-bot code to see if it's using fraction conversions somewhere, and possibly converting them to strings using Fraction.toString() at default precision.

If we use only bignumber.js for unit conversions at default precision, I think the tiny errors go away.

PseudoDeterminist commented 5 years ago

You wrote:

converting to Number is a measure of last resort, only for experienced users!

But a line like this one, line 103 in exchange.js

let parsedAmount = new Fraction(amount * price).mul(new Fraction(10).pow(etherDecimals))

is making the same mistake.

That line is within this function beginning at line 93):

private async newBuyOrder(tokenAddress: string, amount: number, price: number, orderContract: string) : Promise<string> { ....

here amount is a number, price is a number.

So in node, as an example:

parsedAmount = new Fraction(123.2537 * .00004).mul(new Fraction(10).pow(18)) { s: 1, n: 1.4964e+22, d: 3035203 }

Hell of a numerator and denominator there!

parsedAmount.toString() '-2009419937.095319818806188'

Here we get complete junk, but note we did not have to do toString() to get the error.

calling Fraction on plain numbers will do it.

BigNumber shows us what we did there:

BigNumber('1.4964e22').dividedBy(BigNumber(3035203)) 4930147999985503.44079127491637297406

That number should be:

BigNumber('123.2537').times(BigNumber('.00004')).times('1e18') 4930148000000000

Found it!!! And bignumber.js is our friend for fixing it.

PseudoDeterminist commented 5 years ago

My assumption that Fraction.js had "infinite" precision was also wrong.

From https://github.com/infusion/Fraction.js/

Precision Fraction.js tries to circumvent floating point errors, by having an internal representation of numerator > and denominator. As it relies on JavaScript, there is also a limit. The biggest number representable is > |Number.MAX_SAFE_INTEGER / 1| and the smallest is |1 / Number.MAX_SAFE_INTEGER|, with Number.MAX_SAFE_INTEGER=9007199254740991.

According to their web page, the above 16 digit number is the largest integer they can handle, and they do not implement a way of increasing it.

I may have missed something on their web page, but that's kind of what it looks like it is saying.

That's not enough precision in Fraction to fix this issue.

I've tested in node and it's true. Fraction at default levels, which exchange.ts is using, can't handle ordinary wei amounts, even to 16 places. Let alone 18 or higher, which we routinely use.

It tries, which is why exchange.ts has been "mostly" working! But its accuracy is less than 16 significant digits.

I'm thinking you must have made the same assumption of "infinite" precision that I did.

I want to say that I am very interested in doing a first pull request with Saturn Network. And I will!

But first I want to pause here and ask for feedback.

At least, if I am right about Fraction, bignumber.js has one answer! It works better than ethers.js for units conversion and just lots of things. It can indeed do everything we need.

saturn-network commented 5 years ago

re: Fraction, good to know!

We use Fraction in js because of how different languages deal with numbers. Solidity (the smart contracts), as you mentioned, only deal with integers. In plain English we prefer to deal with floats when we discuss prices and amounts. E.g. we prefer saying "I bought 1 $ETH worth of $X at 0.42 price".

Javascript acts as glue between smart contracts that execute the trades, and English, the language we use to describe our intents. The sad thing about JS is that its floating point numbers suck (per IEEE standard, that's how your CPU works), and solidity doesn't even have floating points. Thus, every order is being set up with two parameters under the hood: priceMul and priceDiv. Then, the smart contract does this: tokenAmount = etherAmount.mul(priceMul).div(priceDiv). Note that the differences in decimals get accounted for in the price as well, e.g. it should really be called weiAmount internally...

Ok, this story aside, let's get back to business. And business tells us that once we find a price that we like, represented by a floating point that we like, we can use the fact that rational numbers are dense in the reals in order to approximate any desired price with a rational number in the form of priceMul / priceDiv. This is what we used Fraction.js for. If it is not doing its job well, then we should replace this library with something that satisfies our requirements.

re: bignumber.js - what an amazing library!

If you can figure out how to use it in order to convert any floating point number to a rational number representation that would solve our pricing issues. As far as ethers.js goes, unfortunately it has its own version of bignumber math and we need to do bignumber -> string, e.g. .toFixed() -> ethers.bignumber.

PseudoDeterminist commented 5 years ago

Yes, I would love to work with you on completely eliminating Fraction.js from saturn.js. It's only slightly unfortunate that ethers.js likes the word BigNumber, and so does bignumber.js.

I'd like to immediately import bignumber.js into exchange.ts with some new name, like

const BigNumberJS = require('bignumber.js')

and start working on getting rid of Fraction.

That's a big job, but if you are interested I'll submit a pull request with an incremental small change to exchange.ts using bignumber.js in just one place and see how it tests out with you.

I'm thinking line 103 is the place to start!

PseudoDeterminist commented 5 years ago

If you can figure out how to use it [bignumber.js] in order to convert any floating point number to a rational number representation that would solve our pricing issues.

Not a problem. But I would also like to look into using modular arithmetic to handle some pricing issues.

Amazingly, bignumber.js also "appears" (I have to really look and see) to support modulo division with remainder. If so, not only can we figure out how many tokens the user can buy with his wei, but also exactly how much wei to return to him if he sends too much. Much simpler than using fractions.

Just a thought. I'm going to actually get to work on exchange.ts with bignumber.js now.

saturn-network commented 5 years ago

That's a big job, but if you are interested I'll submit a pull request with an incremental small change to exchange.ts using bignumber.js in just one place and see how it tests out with you.

The best way would be for you to develop your own module that acts as a plug-and-play replacement of Fraction.js, and then we can just import that module into our packages.

Amazingly, bignumber.js also "appears" (I have to really look and see) to support modulo division with remainder. If so, not only can we figure out how many tokens the user can buy with his wei, but also exactly how much wei to return to him if he sends too much. Much simpler than using fractions.

The need is dictated by solidity, not by js. No javascript library, not even bignumber.js, will give us floating point numbers in Solidity.

PseudoDeterminist commented 5 years ago

The best way would be for you to develop your own module that acts as a plug-and-play replacement of Fraction.js, and then we can just import that module into our packages.

Fraction.js would be totally not needed even if it were "infinite" precision as we both expected. I am hoping to talk you into using modular math everywhere. No fractions!

The need is dictated by solidity, not by js. No javascript library, not even bignumber.js, will give us floating point numbers in Solidity.

Solidity undoubtedly has modular division with remainder. That is the correct method for blockchain work. We will simply make saturn.js match it. No floating point required!

The reason we like bignumber.js is because if we want to think in floating point in javascript, we can. This amazing package, however, can bridge the gap between Solidity and javascript. I checked: all the modular math we could hope for, to match Solidity contracts. We can even do "modular math" on decimals past the decimal point. Any way you like. Users can type in .0001 and bignumber.js will understand this exactly, no rounding errors. We can easily convert from that to ethers.js BigNumber, no rounding errors, with just a BigNumberJS.shiftedBy(decimals) call. And shiftedBy(-decimals) converts back, for output to user.

Future is bright!

saturn-network commented 5 years ago

Fraction.js would be totally not needed even if it were "infinite" precision as we both expected. I am hoping to talk you into using modular math everywhere. No fractions!

5 addresses this, good find!

saturn-network commented 5 years ago

Bounty: 69,000 SATURN tokens of your choosing courtesy of Neuron.

I think we know the name of the winner

saturn-network commented 5 years ago

This issue is still not resolved btw, but a lot of info has been published on how to tackle it. An easy opportunity to learn, contribute to an open source project, and earn some $SATURN.