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

Add a whole balance limit to UserDeposit #678

Merged
merged 3 commits into from
Mar 5, 2019

Conversation

pirapira
Copy link
Contributor

@pirapira pirapira commented Mar 5, 2019

which is configurable at the deployment time.

Currently we keep the option of limiting deposits in all smart contracts (https://github.com/raiden-network/team/issues/324#issuecomment-465100567).

I chose against using the contract's token balance because that poses a DOS attack possibility.

This closes #585.

@pirapira pirapira force-pushed the userdeposit_limit branch 3 times, most recently from c6b0536 to 7b345ce Compare March 5, 2019 16:28
@pirapira pirapira requested review from palango and karlb March 5, 2019 16:51
@karlb
Copy link
Contributor

karlb commented Mar 5, 2019

I'm not sure which requirements we have for the choice of our limits. Having a per-account limit would be simpler and more gas efficient than having a global limit. But I assume you know that and did it this way for a good reason.

@pirapira
Copy link
Contributor Author

pirapira commented Mar 5, 2019

@karlb the reason for the total limit can be seen here: https://github.com/raiden-network/team/issues/333

In general, one person can create an unlimited number of Ethereum accounts, so usually per-account limitations are not very effective.

pirapira added 2 commits March 5, 2019 20:25
which is configurable at the deployment time.

Currently we keep the option of limiting deposits in all smart
contracts.
@pirapira pirapira force-pushed the userdeposit_limit branch from 7b345ce to b596f66 Compare March 5, 2019 19:54
@codecov
Copy link

codecov bot commented Mar 5, 2019

Codecov Report

Merging #678 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #678   +/-   ##
=======================================
  Coverage   20.29%   20.29%           
=======================================
  Files          16       16           
  Lines        1069     1069           
  Branches      118      118           
=======================================
  Hits          217      217           
  Misses        850      850           
  Partials        2        2

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 12961be...b596f66. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 5, 2019

Codecov Report

Merging #678 into master will increase coverage by 44.49%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #678       +/-   ##
===========================================
+ Coverage   20.29%   64.79%   +44.49%     
===========================================
  Files          16       17        +1     
  Lines        1069     1122       +53     
  Branches      118      119        +1     
===========================================
+ Hits          217      727      +510     
+ Misses        850      369      -481     
- Partials        2       26       +24
Impacted Files Coverage Δ
raiden_contracts/deploy/__main__.py 67.75% <83.33%> (+67.75%) ⬆️
setup.py 0% <0%> (ø)
raiden_contracts/utils/private_key.py 25% <0%> (+25%) ⬆️
raiden_contracts/utils/signature.py 88% <0%> (+32%) ⬆️
raiden_contracts/utils/transaction.py 65.21% <0%> (+43.47%) ⬆️
raiden_contracts/utils/proofs.py 91.89% <0%> (+45.94%) ⬆️
raiden_contracts/contract_manager.py 78.16% <0%> (+55.63%) ⬆️
raiden_contracts/utils/merkle.py 82.35% <0%> (+55.88%) ⬆️
raiden_contracts/utils/logs.py 87.5% <0%> (+68.26%) ⬆️
... and 3 more

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 12961be...a110d6c. Read the comment docs.

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.

Add deposit limit to UserDeposit
2 participants