Conversation
removed the tokenPricePrecision property from assets use priceDecimals rather than price precision (use 4, not 10**4) provide priceDecimals as a parameter while creating campaign provide priceDecimals as a parameter while updateing price on ApxRegistry (important for liquidations) track priceDecimals when storing the highestSellPrice on asset state (important for liquidations) make all the _token_value() methods look the same (operations ordering) fix tests add priceDecimalsPrecision to the campaign common state fix deployer service scripts to take into account new priceDecimals parameter when creating campaigns
SD-601 Remove tokenPricePrecision from Assets!
tokenPricePrecision is used when calculating the value of tokens when reserving the investment on campaign contracts. Requirement:
|
| IToken stablecoin, | ||
| uint256 price | ||
| uint256 price, | ||
| uint256 priceDecimals |
There was a problem hiding this comment.
Why use uint256 for decimals?!
There was a problem hiding this comment.
It's a read-only service so any optimization is pointless but I'll fix it in the new commit with other changes
| function _token_amount_for_investment( | ||
| uint256 investment, | ||
| uint256 tokenPrice, | ||
| uint256 tokenPriceDecimals, |
There was a problem hiding this comment.
Why use uint256 for decimals?!
| _safeFinalizeSale(); | ||
| _safeDistributeFunds(msg.sender, fundsRaised, sc); |
There was a problem hiding this comment.
Why is a safe transfer needed?
There was a problem hiding this comment.
If for any reason FeeManager interaction fails we still want to allow the campaign manager to finalize the campaign , or else he's locked out of his funds. In other words, we want the fee calculation to silently fail
| @@ -70,7 +70,7 @@ contract Structs { | |||
| address issuer; | |||
| address paymentMethod; | |||
There was a problem hiding this comment.
Can we rename it to something like paymentToken or paymentTokenAddress or something else?
as word method tells "how" something is paid (e.g. cash, card, "u naturi").
| @@ -52,7 +52,7 @@ contract Structs { | |||
| address issuerAddress; | |||
| address paymentMethod; | |||
|
thx !
AMPNET
Filip Dujmušić, COO
Phone: +385 95 891 5667
Email: ***@***.***
Web: https://ampnet.io
AMPnet IO d.o.o., Josipa Marohnica 3, 10000 Zagreb. VAT Registration:
HR57624879969.
Please consider the environment before printing this message.
…On Tue, Mar 1, 2022 at 4:10 PM Eugen Druzin ***@***.***> wrote:
***@***.**** approved this pull request.
—
Reply to this email directly, view it on GitHub
<#47 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJFWXD5I3JCVHN65MHGTQE3U5YXNBANCNFSM5PUFZFDA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Alles gut, but check whether it works in frontend branch. demo is here.
Check all screens related to assets and campaigns. Try to create a new asset and campaign to see how it works. Keep the console opened to see possible errors. gl hf
removed the tokenPricePrecision property from assets
use priceDecimals rather than price precision (use 4, not 10**4)
provide priceDecimals as a parameter while creating campaign
provide priceDecimals as a parameter while updateing price on ApxRegistry (important for liquidations)
track priceDecimals when storing the highestSellPrice on asset state (important for liquidations)
make all the _token_value() methods look the same (operations ordering)
fix tests
add priceDecimalsPrecision to the campaign common state
fix deployer service scripts to take into account new priceDecimals parameter when creating campaigns