Skip to content
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

Merged
merged 3 commits into from
Mar 4, 2021

Conversation

palango
Copy link
Contributor

@palango palango commented Feb 24, 2021

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.

@palango palango force-pushed the lib-split-test branch 13 times, most recently from 4797132 to 4cd77d5 Compare March 1, 2021 17:09
@palango palango changed the title WIP: Lib split test Minimal code splitting for TokenNetwork - using a library for pure functions Mar 1, 2021
@palango
Copy link
Contributor Author

palango commented Mar 1, 2021

@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.

@palango palango requested a review from karlb March 2, 2021 09:16
Copy link
Contributor

@karlb karlb left a 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.

.github/workflows/python-package.yml Outdated Show resolved Hide resolved
.github/workflows/python-package.yml Outdated Show resolved Hide resolved
@palango palango force-pushed the lib-split-test branch 2 times, most recently from 5db344c to b2670cd Compare March 2, 2021 12:09
@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #1428 (7c48dce) into master (e268bd7) will increase coverage by 0.35%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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              
Impacted Files Coverage Δ
raiden_contracts/constants.py 100.00% <100.00%> (ø)
raiden_contracts/deploy/contract_deployer.py 89.28% <100.00%> (+1.09%) ⬆️
raiden_contracts/deploy/contract_verifier.py 84.71% <100.00%> (+0.29%) ⬆️
raiden_contracts/utils/linking.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e268bd7...7c48dce. Read the comment docs.

@palango palango force-pushed the lib-split-test branch 2 times, most recently from c19aa58 to 0b2ead1 Compare March 2, 2021 12:31
@palango palango marked this pull request as ready for review March 2, 2021 16:27
@auto-assign auto-assign bot requested a review from konradkonrad March 2, 2021 16:27
@palango
Copy link
Contributor Author

palango commented Mar 2, 2021

The etherscan verification are still WIP, but it can be pushed into a follow-up when necessary. The rest is good for review.

raiden_contracts/deploy/contract_verifier.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimal code splitting for TokenNetwork - using a library for pure functions
2 participants