Market maker review #3
Conversation
| require(outcomeTokenDistribution.length > 1); | ||
| uint[2] memory outcomeTokenRange = getOutcomeTokenRange(outcomeTokenDistribution); | ||
| uint invB = uint(Math.ln(outcomeTokenDistribution.length * ONE)) / 10000; | ||
| require(market.eventContract().getOutcomeCount() > 0); |
| public | ||
| returns(uint costs) | ||
| /// @return Returns costLevel | ||
| function calcCostFunction(uint logN, int[] netOutcomeTokensSold, uint funding) |
There was a problem hiding this comment.
Rename to calcCostLevel
| // Math.safeToSub(netOutcomeTokensSold[outcomeTokenIndex], outcomeTokenCount) && | ||
| int(outcomeTokenCount) >= 0 | ||
| ); | ||
| netOutcomeTokensSold[outcomeTokenIndex] -= int(outcomeTokenCount); |
There was a problem hiding this comment.
Replace with overloaded Math.sub
| // Math.safeToAdd(netOutcomeTokensSold[outcomeTokenIndex], outcomeTokenCount) && | ||
| int(outcomeTokenCount) >= 0 | ||
| ); | ||
| netOutcomeTokensSold[outcomeTokenIndex] += int(outcomeTokenCount); |
There was a problem hiding this comment.
Replace with overloaded Math.add
There was a problem hiding this comment.
line 71: self.assertFalse(self.math.safeToAdd(2**256 - 1, 1))
in utils/test_math.py fails because the test assumes unsigned integers.
There was a problem hiding this comment.
Oh yes, this is going to be tricky... to be honest, I wasn't sure how the tests would handle overloading in ambiguous cases. I'm guessing the current behavior is to just pick one version and stick with it. There may be a way to test it, but I'm not sure if changes need to be made at the pyethereum level or the AbstractTestContracts level.
Do you mind pushing up a branch for me to look at?
There was a problem hiding this comment.
This is an issue with pyethereum. I opened an issue for this long time ago: ethereum/pyethereum#418
How does truffle handle this? Do you know?
There was a problem hiding this comment.
Truffle does handle this situation with Solidity tests. JS tests cause some Unspecified AssertionError
There was a problem hiding this comment.
Lets comment out this assertion and add a comment, why this currently not works.
| profits = (costsBefore - costsAfter) * (funding / 10000) * (100000 - 2) / 100000 / ONE; | ||
| require(costLevelBefore >= costLevelAfter); | ||
| // Take the floor | ||
| profits = uint(costLevelBefore - costLevelAfter) / ONE; | ||
| } | ||
|
|
||
| /// @dev Returns current price for given outcome token |
There was a problem hiding this comment.
Make this more accurate
|
|
||
| require( | ||
| int(outcomeTokenCount) >= 0 && | ||
| netOutcomeTokensSold[outcomeTokenIndex] + int(outcomeTokenCount) >= netOutcomeTokensSold[outcomeTokenIndex] |
There was a problem hiding this comment.
Factor into safeToAdd/add
|
|
||
| require( | ||
| int(outcomeTokenCount) >= 0 && | ||
| netOutcomeTokensSold[outcomeTokenIndex] - int(outcomeTokenCount) <= netOutcomeTokensSold[outcomeTokenIndex] |
There was a problem hiding this comment.
Factor into safeToSub/sub
| return b * math.log( | ||
| sum([math.exp(share_count / b + token_count / b) for share_count in token_distribution]) - | ||
| sum([math.exp(share_count / b) for index, share_count in enumerate(token_distribution) if index != token_index]) | ||
| ) - token_distribution[token_index] |
There was a problem hiding this comment.
Consider putting back, but at least implement it down the line and test with gnosis.js
| (10, 5), | ||
| (10*10**18, 5000), | ||
| (10*10**18, 5), | ||
| (10, 5 * 10 ** 18), |
There was a problem hiding this comment.
Use self.s.block.set_balance to make some extreme tests
|
|
||
| for x in chain( | ||
| (MIN_POWER, -497882689251500345055, -1), | ||
| (random.randrange(MIN_POWER, 0) for _ in range(10)), |
There was a problem hiding this comment.
Could potentially factor these back up into the normal domain test cases
|
Try changing the type of it everywhere.
…On Wed, May 31, 2017 at 2:55 PM, Collin Chin ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In contracts/solidity/MarketMakers/LMSRMarketMaker.sol
<#3 (comment)>:
> {
- uint[] memory outcomeTokenDistribution = getOutcomeTokenDistribution(market);
- require(outcomeTokenDistribution.length > 1);
- uint[2] memory outcomeTokenRange = getOutcomeTokenRange(outcomeTokenDistribution);
- uint invB = uint(Math.ln(outcomeTokenDistribution.length * ONE)) / 10000;
+ require(market.eventContract().getOutcomeCount() > 0);
+ int[] memory netOutcomeTokensSold = getNetOutcomeTokensSold(market);
+
+ uint logN = uint(Math.ln(netOutcomeTokensSold.length * ONE));
Needs cast. Throws Error: Type int256 is not implicitly convertible to
expected type uint256. otherwise
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASNzxM0PNZ8R_G6g14BF5caLzWsmlO0ks5r_cXGgaJpZM4Nqrbx>
.
|
No description provided.