Uniswap demo #179
Uniswap demo #179
Conversation
Codecov Report
@@ Coverage Diff @@
## master #179 +/- ##
==========================================
+ Coverage 77.93% 77.96% +0.03%
==========================================
Files 45 45
Lines 3300 3300
==========================================
+ Hits 2572 2573 +1
+ Misses 728 727 -1
Continue to review full report at Codecov.
|
g-r-a-n-t
force-pushed
the
uniswap-contracts
branch
2 times, most recently
from
04051cd
to
9bc1ae0
Compare
|
Still a few small things I want to clean up and features that need to be documented, but I thought it would be nice to get some feedback before the weekend. |
| pairs: map<address, map<address, address>> | ||
|
|
||
| # todo: discuss unbounded storage arrays | ||
| all_pairs: address[0] |
There was a problem hiding this comment.
Good point. I've been thinking about this every now and then
|
|
||
| # update reserves and, on the first call per block, price accumulators | ||
| def _update(balance0: u256, balance1: u256, reserve0: u112, reserve1: u112): | ||
| assert balance0 <= u112(-1) and balance1 <= u112(-1), "UniswapV2: OVERFLOW" |
There was a problem hiding this comment.
I dont think we want to get max values using u112(-1)
There was a problem hiding this comment.
Yeah, that's a good point. It feels hacky to me that some languages even allow this. We already have a check in place that something like u8(-1) doesn't compile (nothing of type uxxx allows passing -1). I think we want something like u8.max(), u8.min() to expose these.
There was a problem hiding this comment.
Yeah I like the idea of having uxxx.max() & uxxx.min().
g-r-a-n-t
force-pushed
the
uniswap-contracts
branch
5 times, most recently
from
204b072
to
1d2d476
Compare
|
@cburgdorf , core functionality seems to work. There are a few more things that we should try to get done before April, tho. These things are documented in TODO comments that link to issues. |
There was a problem hiding this comment.
Really really cool! I left a few comments regarding things that I consider worth to create issues for and link to them in the example. I also think we should put the testing code into a separate file because our all-tests-in-one-file approach is starting to look unsustainable 😅
Other than that, looks amazing 👍
g-r-a-n-t
force-pushed
the
uniswap-contracts
branch
from
1d2d476
to
8882959
Compare
g-r-a-n-t
force-pushed
the
uniswap-contracts
branch
from
8882959
to
9155926
Compare
What was wrong?
We need some contracts for the Uniswap demo.
How was it fixed?
To-Do
OPTIONAL: Update Spec if applicable
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history