-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minimal code splitting for TokenNetwork - using a library for pure functions #1428
Conversation
6de076e
to
8fa65ac
Compare
4797132
to
4cd77d5
Compare
@karlb This passes all but two tests now. I'm working on those. If you want you can already give this a first look and provide some feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far! This will probably make changes to the etherscan verififaction necessary, too.
5db344c
to
b2670cd
Compare
Codecov Report
@@ Coverage Diff @@
## master #1428 +/- ##
==========================================
+ Coverage 80.27% 80.63% +0.35%
==========================================
Files 21 22 +1
Lines 1516 1544 +28
Branches 187 191 +4
==========================================
+ Hits 1217 1245 +28
Misses 253 253
Partials 46 46
Continue to review full report at Codecov.
|
c19aa58
to
0b2ead1
Compare
The etherscan verification are still WIP, but it can be pushed into a follow-up when necessary. The rest is good for review. |
Fixes #401
This PR splits the a first pure function out of the token network contract into an library. It also implements the necessary infrastructure for doing this, like deploying and linking the library into the token network contract code.
I did not yet pull all possible code into the library in order to make this PR smaller and easier to review. Once this is merged it should be easy to do this and reduce the contract code size further.
I also decided to only create a library for the token network contract now, as the code size gains seems small compared to the added complexity from deploying/linking for the other contracts look unworthy.