closerdao / proof-of-presence

TDF smart contracts
MIT License
2 stars 2 forks source link

(DynamicPriceSale.sol) Test new crowdsale math function and update tests #39

Open juliencharrel opened 1 year ago

juliencharrel commented 1 year ago

Work already started in the sale_contract branch. Start from there.

I updated the _calculatePrice function that is supposed to compute 1/ the last price after increase of supply 2/The cost of buying tokens (delta cost between a start and end supply).

You need to:

https://docs.google.com/spreadsheets/d/17PEOgTeItXcquVjt1hvyfUIXu9y0ZHjsAsUpS3DhWFA/edit#gid=2075084444 Image

marceljay commented 1 year ago

1) _calculatePrice is supposed to calculate the new price, right?

2) Why do parameters a and b need to be such high numbers with over 46 digits? I don't see wy that would be necessary.

3) Is the TDF token going to be a 18-decimal ERC20?

juliencharrel commented 1 year ago
  1. Yes
  2. It's because I scaled the formula so you can use wei in it (the initial formula was made for 1 ether/token). The initial formula had smaller numbers
  3. Yes
marceljay commented 1 year ago
  1. Why did you put 10^36 though? I don't think the values must be so high, we are talking here about astronimically small numbers, i don't see the purpose. Since we already deal with 10^18 decimals, i argue we can reduce the factors by large.
juliencharrel commented 1 year ago

It's because we need to scale the parameters and our formula has ^2 and ^3 parameters in it. Look into the spreadsheet here https://docs.google.com/spreadsheets/d/17PEOgTeItXcquVjt1hvyfUIXu9y0ZHjsAsUpS3DhWFA/edit#gid=2075084444 and here's a screenshot of side by side formula for 1 ether of units vs 1 wei

image

If you want to use the other formula, you would need to do calculation with ether and then handle float numbers, that's why I scaled the cost and price functions

What is the problem with large numbers? (as long as we guarantee no overflow)

marceljay commented 1 year ago

Ok sure, I understand now why the factors needed to be scaled.

But right now I actually wonder if we need to operate in wei-style decimals because it seems to be a requirement that no fractional TDF tokens can be bought.

        require(amount % 1 ether == 0, "DynamicSale: (NonWholeUnit) only whole units allowed");

What do you think?

juliencharrel commented 1 year ago

Good catch, you can remove that requirement ;)

arturictus commented 1 year ago

There is an script to generate a Json file from the pricing output. We should generate this file, confirm with Tokenomics team and test that the contract outputs the same results in tests.

sorokinvj commented 1 year ago

hey guys, I am trying to use scripts/output_pricing.ts, but it's failing and not giving me the json I need for the test. I believe this is because there is a Typescript error in lines 43:44:

nextUnitPrice: formatEther(await contracts.sale.calculateTotalCost(parseEther('1'))),
amountCost: formatEther(await contracts.sale.calculateTotalCost(parseEther(step.toString()))),

so calculateTotalCost is returning a type [BigNumber, BigNumber] & { newPrice: BigNumber; totalCost: BigNumber } which: a) looks like a union of quite different types, is this expected? b) is definitely not good for the formatEther argument that expects type BigNumberish = BigNumber | Bytes | bigint | string | number

@arturictus @juliencharrel do you guys have any idea how to fix output_pricing.ts?

arturictus commented 1 year ago

You can use the output like an object output.totalCost and the format should pick it up

sorokinvj commented 1 year ago

@arturictus Artur, sorry, but I am lost.

  1. Where do I get the output object, that you mentioned? Is it something that exists on contracts.sale (I don't see it there)?
  2. When we sort out the output_pricing.ts and we get the json, how do I insert the data into a test, would it be something like:
    
    // suppose json have arrays of {amount, price)

json.forEach(arrayItem => await user.calculateTotalCost(arrayItem.amount).toEq(arrayItem.price));


☝️ is this what we want to test?
arturictus commented 1 year ago

// suppose json have arrays of {amount, price)

json.forEach(arrayItem => await user.calculateTotalCost(arrayItem.amount).totalCost.toEq(arrayItem.price));

notice: .totalCost.toEq(

sorokinvj commented 1 year ago

wait, I just realized my question doesn't make sense, I compare calculateTotalCost to itself, obviously, this is going to work:)

I am sorry for the confusion, @arturictus could we please align on the task:

  1. We need control data, our source of truth - what is it? Column B (purchasing price) from excel? Which is a function from Column A (amount), fn: A -> B?
  2. We need the function we want to test - is it calculateTotalCost from DynamicSale contract?
  3. So test is basically calculateTotalCost(column A) == column B

Do you know if my understanding of the task is correct?

I opened a pr with my naive implementation

sorokinvj commented 1 year ago

assigning this task back to @arturictus as I failed again to accomplish it.