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

feat: atomic ntoken #326

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

feat: atomic ntoken #326

wants to merge 19 commits into from

Conversation

GopherJ
Copy link
Contributor

@GopherJ GopherJ commented Jan 31, 2023

Signed-off-by: GopherJ [email protected]

Security Checklist

  • 1. Re-Entrancy
  • 2. Arithmetic Over/Under Flows
  • 3. Unexpected Ether
  • 4. Delegatecall
  • 5. Default Visibilities
  • 6. Entropy Illusion
  • 7. External Contract Referencing
  • 8. Short Address/Parameter Attack (off chain)
  • 9. Unchecked CALL Return Values
  • 10. Race Conditions / Front Running
  • 11. Denial Of Service (DOS)
  • 12. Block Timestamp Manipulation
  • 13. Constructors with Care
  • 14. Uninitialized Storage Pointers
  • 15. Floating Points and Precision
  • 16. Tx.Origin Authentication
  • 17. Address.isContract Re-Entrancy via Constructor

⚠️ NOTES ⚠️

Make sure to think about each of these exploits in this PR.

Signed-off-by: GopherJ <[email protected]>
@GopherJ GopherJ requested a review from a team as a code owner January 31, 2023 14:19
Signed-off-by: GopherJ <[email protected]>
Signed-off-by: GopherJ <[email protected]>
@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Merging #326 (e9c2b17) into main (fcb705c) will increase coverage by 0.55%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
+ Coverage   75.24%   75.80%   +0.55%     
==========================================
  Files          83       83              
  Lines        3826     3918      +92     
  Branches      714      737      +23     
==========================================
+ Hits         2879     2970      +91     
+ Misses        571      560      -11     
- Partials      376      388      +12     
Impacted Files Coverage Δ
...ntracts/protocol/tokenization/NTokenApeStaking.sol 74.19% <ø> (ø)
...ontracts/protocol/tokenization/NTokenUniswapV3.sol 7.40% <20.00%> (+3.24%) ⬆️
contracts/ui/UiPoolDataProvider.sol 59.64% <33.33%> (-0.24%) ⬇️
...acts/protocol/libraries/logic/LiquidationLogic.sol 97.03% <66.66%> (+0.02%) ⬆️
...ontracts/protocol/tokenization/NTokenMoonBirds.sol 68.42% <66.66%> (ø)
contracts/protocol/tokenization/NToken.sol 36.92% <83.33%> (-0.96%) ⬇️
...l/tokenization/base/MintableIncentivizedERC721.sol 65.68% <83.33%> (+9.84%) ⬆️
...col/tokenization/libraries/MintableERC721Logic.sol 83.41% <92.10%> (+4.99%) ⬆️
contracts/protocol/libraries/helpers/Helpers.sol 100.00% <100.00%> (ø)
...ontracts/protocol/libraries/logic/GenericLogic.sol 70.29% <100.00%> (+3.94%) ⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

{
return (
_ERC721Data.userState[account].balance,
_ERC721Data.userState[account].atomicBalance,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should optimize the gas cost here because we are not using all these anymore in GenericLogic

@@ -46,6 +46,7 @@ struct MintableERC721Data {
uint64 balanceLimit;
mapping(uint256 => bool) isUsedAsCollateral;
mapping(uint256 => DataTypes.Auction) auctions;
address underlyingAsset;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should still take the same slot 67

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.

2 participants