-
Notifications
You must be signed in to change notification settings - Fork 56
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
Mint next account #1019
Mint next account #1019
Conversation
Codecov Report
@@ Coverage Diff @@
## feature-v3-mvp #1019 +/- ##
===============================================
Coverage 97.77% 97.77%
===============================================
Files 98 98
Lines 1527 1527
Branches 156 156
===============================================
Hits 1493 1493
Misses 34 34
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
} | ||
|
||
function mintNext(address accountOwner) external override returns (uint256 accountId) { | ||
while (_exists(_accountStore().recentIdUsed)) { |
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.
you can check before if the number of tokens created is less than recentIdUsed and fast forward to it, also write on the while loop to an internal variable and at the end go to the store since it's cheaper.
i.e.
if (checkId < _accountStore().balance) {
checkId = _accountStore().balance;
}
while (_exists(checkId) {
checkId++;
}
accountId = checkId;
_accountStore().recentIdUsed = accountId;
and you need to add the balance in the store (uint256 balance,
) and an increment to the public mint function (_accountStore().balance++;
)
That will save some gas if there are mixed usage of mint()
and mintNext()
from contracts, direct access or alternative UIs
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.
Is there a way to rely on totalSupply instead? Also, I don't think the total tokens created would ever be less than recentIdUsed? (If mint is used directly, the total tokens would be greater. We wouldn't necessarily want to 'fast-forward' and instead 'fill in' the intermediate values I think?)
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.
Added a suggestion (optional) and committed a small change (adjust variable names in the Interface to match the contract).
This reverts commit 4edbdd5.
This reverts commit 4edbdd5.
* add controlled mint/burn to synth * Abstract authorization and propagate to SNX and Synth * Use authorization to mint * Bootstrap AccountModule (#895) * Add permission management to account * (WIP) Implement permissions based modifier for account * increase packed permission store * Improve Permission identification * cleanup * Add Account delegation operations * Set as Satellite and add Module test * lint * cleanup * more cleanup * rogue copypasta * add draft API for account and fund * reduce PR to just bootstrap the module and satellite * Bootstrap FundModule (#897) * initial commit * cleanup satellite stuff * V3-mvp/base-account-and-fund-interface (#904) * initial commit * move error definition to AccountBase * added basic stuff for fund * add some comments * remove dependency on accountBase for the moment * wip - add more logic to account * more updates * cleanup Account * define basics fund storage * Make it compile and deploy * basic SIP interface coverage * pair programming storage and some logic * Account Code compiling * Add enabled Collateral Types * bootstrap test * some logs cleanup * extend deploy test timeout * add some tests structure * staking and unstaking happy path * cleanup * lint * add more tests * V3-mvp/fund (#926) * initial commit * more coding... * fix some errors * make it compile * correct transfer ownership signature on fund token * some accounting logic * Add more logic and make it compile * remove duplicated function * Start CollateralModule migration from account * add collateralModule test * refactor WIP * (WIP) is deployable * add some comments to Collateral Module * split account test * add RBAC tests to account * bootstrap RBACMixin test * complete tests for RBAC Mixin * Complete Collateral Module SCCP test * Add missing tests to AccountModuleToken * Fix CollateralModule Stake * Lint + cleanup * fundToken prevent transferFrom and allow coordinated ownership transfer * Test for FundModule Token (Ownership) * remove link between the owner on the FundToken and FundModule * bootstrap funds (functionality) test * more tests and get fund positions * Add tests to cover one account liquidity changes * some cleanup * lint * Collateral SCCP fixed * some fixes from feedback * make tests pass * Fund SCCP add coverage, some errors and events * cleanup some TODOs * add collateralMixin * Lint + make tests pass again * add code for minting and burning sUSD * clean up some variables * add comments and reorder * duh * merge * add multicall module, finalize initial cannonfile * update package-lock * update deploy script to use new multicall abi task (#1008) * Add MulDivUp Math util (#1015) * add utils * lint * PR fix * lint * Change AccountModule.createAccount to AccountToken.mint (#1016) * Initial commit * fix tests * Add views for account liquidity items (#1020) * first commit * add missing hidratation * Mint next account (#1019) * mintNext function * fix lint * Apply suggestions from code review Co-authored-by: Leonardo Massazza <lmassazza@gmail.com> * fix issues with deployment and oracle price resolve * Fixes for onboarding (#1027) * Revert "Mint next account (#1019)" This reverts commit 4edbdd5. * Revert "Change AccountModule.createAccount to AccountToken.mint (#1016)" This reverts commit f5fcf54. * upgrade cannon * Split Fund Module (#1024) * Initial commit * make tests pass * add sUSD token and mint/burn from fund * pr updates * fix test * make test work * rename FundSCCP to FundConfiguration * update getCollateralTypes etc * Rename stuff and fix tests * market module WIP * fix tests - add setAllowance * lint * wip * wip rename sUSD to USD * started liquidations * wip * wip * move curve to library; make it compile * make tests great again * change return value to CollateralData * implement registerMarket * improve MathUtil * wip * update cannon version * update cannon version * link Fund to MarketManager * fix test * add tests to MathUtil lib * overload functions for uint and int * cleanup and order parameters * Remove supplyTarget and order/rename variables * Document interfaces * Cannonfile Update (#1038) * fixed first issue * fix cannonfile build * v 0.7 * version fix * progress * working * start testing MarketManager * Add test * fix package lock * add fundMaxDebtShare (supplyTarget cap) * add functionality to MarketMock * bootstrap whole system test * more tests * update cannon version * add associated systems module simplify the implementation of satellites to the leanest level. also more cannon friendly * cannon update * Market module (#1033) * market module WIP * fix tests - add setAllowance * lint * wip * wip rename sUSD to USD * started liquidations * wip * wip * move curve to library; make it compile * make tests great again * change return value to CollateralData * implement registerMarket * improve MathUtil * wip * update cannon version * link Fund to MarketManager * fix test * add tests to MathUtil lib * overload functions for uint and int * cleanup and order parameters * Remove supplyTarget and order/rename variables * Document interfaces * start testing MarketManager * Add test * add fundMaxDebtShare (supplyTarget cap) * add functionality to MarketMock * bootstrap whole system test * more tests * update cannon version Co-authored-by: Noah Litvin <noah.litvin@gmail.com> * lint * Add erc721 enumerability (#1040) * add contract * add hooks * fix prettier * code review fixes * remove interface inheritence * tests * remove only * fix nit * getting close to rewards * add missing files * contracts compile * restructuring of deployment * Cannon fixes (#1042) * cannon and test fix * fix test * make it private * move assocaited systems module to core modules * fix pkglock * update deployment mostly working * changes to tests and add ts time to use typechain and have better tests bootstrap will happen with cannon now (and cannon is cached so tests will be much better) * cannonfile compiles with the new * print info needs to use `getAddress` * changes to core-js to natively support typescript needed to support typescript on downstream tests * more test fixes * working tests * fix coverage * fix lint * remove prettier for now since it doesn't support graceful warnings * more tests fixes * add build step * add another build step * fix syntax * remove sinon types * fix deployer and core module tests * fix * write ts declaration files * working tests * finally works * debt refactor 304 306 compiles * partial * compiles again with theoretically working debt, liquidations, markets * existing tests pass * add some vault tests * tests are passing for 304 markets, and debt! Co-authored-by: Leonardo Massazza <lmassazza@gmail.com> Co-authored-by: Matías <mjlescano@protonmail.com> Co-authored-by: Matías Lescano <mjlescano@users.noreply.github.com> Co-authored-by: Noah Litvin <335975+noahlitvin@users.noreply.github.com> Co-authored-by: Noah Litvin <noah.litvin@gmail.com> Co-authored-by: Sunny Vempati <sunnyvempati@gmail.com>
* Sip 305 rewards (#1045) * add controlled mint/burn to synth * Abstract authorization and propagate to SNX and Synth * Use authorization to mint * Bootstrap AccountModule (#895) * Add permission management to account * (WIP) Implement permissions based modifier for account * increase packed permission store * Improve Permission identification * cleanup * Add Account delegation operations * Set as Satellite and add Module test * lint * cleanup * more cleanup * rogue copypasta * add draft API for account and fund * reduce PR to just bootstrap the module and satellite * Bootstrap FundModule (#897) * initial commit * cleanup satellite stuff * V3-mvp/base-account-and-fund-interface (#904) * initial commit * move error definition to AccountBase * added basic stuff for fund * add some comments * remove dependency on accountBase for the moment * wip - add more logic to account * more updates * cleanup Account * define basics fund storage * Make it compile and deploy * basic SIP interface coverage * pair programming storage and some logic * Account Code compiling * Add enabled Collateral Types * bootstrap test * some logs cleanup * extend deploy test timeout * add some tests structure * staking and unstaking happy path * cleanup * lint * add more tests * V3-mvp/fund (#926) * initial commit * more coding... * fix some errors * make it compile * correct transfer ownership signature on fund token * some accounting logic * Add more logic and make it compile * remove duplicated function * Start CollateralModule migration from account * add collateralModule test * refactor WIP * (WIP) is deployable * add some comments to Collateral Module * split account test * add RBAC tests to account * bootstrap RBACMixin test * complete tests for RBAC Mixin * Complete Collateral Module SCCP test * Add missing tests to AccountModuleToken * Fix CollateralModule Stake * Lint + cleanup * fundToken prevent transferFrom and allow coordinated ownership transfer * Test for FundModule Token (Ownership) * remove link between the owner on the FundToken and FundModule * bootstrap funds (functionality) test * more tests and get fund positions * Add tests to cover one account liquidity changes * some cleanup * lint * Collateral SCCP fixed * some fixes from feedback * make tests pass * Fund SCCP add coverage, some errors and events * cleanup some TODOs * add collateralMixin * Lint + make tests pass again * add code for minting and burning sUSD * clean up some variables * add comments and reorder * duh * merge * add multicall module, finalize initial cannonfile * update package-lock * update deploy script to use new multicall abi task (#1008) * Add MulDivUp Math util (#1015) * add utils * lint * PR fix * lint * Change AccountModule.createAccount to AccountToken.mint (#1016) * Initial commit * fix tests * Add views for account liquidity items (#1020) * first commit * add missing hidratation * Mint next account (#1019) * mintNext function * fix lint * Apply suggestions from code review Co-authored-by: Leonardo Massazza <lmassazza@gmail.com> * fix issues with deployment and oracle price resolve * Fixes for onboarding (#1027) * Revert "Mint next account (#1019)" This reverts commit 4edbdd5. * Revert "Change AccountModule.createAccount to AccountToken.mint (#1016)" This reverts commit f5fcf54. * upgrade cannon * Split Fund Module (#1024) * Initial commit * make tests pass * add sUSD token and mint/burn from fund * pr updates * fix test * make test work * rename FundSCCP to FundConfiguration * update getCollateralTypes etc * Rename stuff and fix tests * market module WIP * fix tests - add setAllowance * lint * wip * wip rename sUSD to USD * started liquidations * wip * wip * move curve to library; make it compile * make tests great again * change return value to CollateralData * implement registerMarket * improve MathUtil * wip * update cannon version * update cannon version * link Fund to MarketManager * fix test * add tests to MathUtil lib * overload functions for uint and int * cleanup and order parameters * Remove supplyTarget and order/rename variables * Document interfaces * Cannonfile Update (#1038) * fixed first issue * fix cannonfile build * v 0.7 * version fix * progress * working * start testing MarketManager * Add test * fix package lock * add fundMaxDebtShare (supplyTarget cap) * add functionality to MarketMock * bootstrap whole system test * more tests * update cannon version * add associated systems module simplify the implementation of satellites to the leanest level. also more cannon friendly * cannon update * Market module (#1033) * market module WIP * fix tests - add setAllowance * lint * wip * wip rename sUSD to USD * started liquidations * wip * wip * move curve to library; make it compile * make tests great again * change return value to CollateralData * implement registerMarket * improve MathUtil * wip * update cannon version * link Fund to MarketManager * fix test * add tests to MathUtil lib * overload functions for uint and int * cleanup and order parameters * Remove supplyTarget and order/rename variables * Document interfaces * start testing MarketManager * Add test * add fundMaxDebtShare (supplyTarget cap) * add functionality to MarketMock * bootstrap whole system test * more tests * update cannon version Co-authored-by: Noah Litvin <noah.litvin@gmail.com> * lint * Add erc721 enumerability (#1040) * add contract * add hooks * fix prettier * code review fixes * remove interface inheritence * tests * remove only * fix nit * getting close to rewards * add missing files * contracts compile * restructuring of deployment * Cannon fixes (#1042) * cannon and test fix * fix test * make it private * move assocaited systems module to core modules * fix pkglock * update deployment mostly working * changes to tests and add ts time to use typechain and have better tests bootstrap will happen with cannon now (and cannon is cached so tests will be much better) * cannonfile compiles with the new * print info needs to use `getAddress` * changes to core-js to natively support typescript needed to support typescript on downstream tests * more test fixes * working tests * fix coverage * fix lint * remove prettier for now since it doesn't support graceful warnings * more tests fixes * add build step * add another build step * fix syntax * remove sinon types * fix deployer and core module tests * fix * write ts declaration files * working tests * finally works * debt refactor 304 306 compiles * partial * compiles again with theoretically working debt, liquidations, markets * existing tests pass * add some vault tests * tests are passing for 304 markets, and debt! Co-authored-by: Leonardo Massazza <lmassazza@gmail.com> Co-authored-by: Matías <mjlescano@protonmail.com> Co-authored-by: Matías Lescano <mjlescano@users.noreply.github.com> Co-authored-by: Noah Litvin <335975+noahlitvin@users.noreply.github.com> Co-authored-by: Noah Litvin <noah.litvin@gmail.com> Co-authored-by: Sunny Vempati <sunnyvempati@gmail.com> * Account permissions view (#1048) * add account permissions * fix cr * fix new references * V3 Docs Workflow (#1051) * for review * lint fix * lint fix Co-authored-by: dbeal <git@danb.email> Co-authored-by: Leonardo Massazza <lmassazza@gmail.com> Co-authored-by: Noah Litvin <335975+noahlitvin@users.noreply.github.com> Co-authored-by: Noah Litvin <noah.litvin@gmail.com> Co-authored-by: Sunny Vempati <sunnyvempati@gmail.com> Co-authored-by: Sunny Vempati <sunny@cc.snxdao.io>
In general, we'd like accounts to be generated with incrementing IDs. We started by allowing accounts to be created at any ID, mostly because in the case of a multicall, you're going to want to be able to queue up later transactions to reference the account ID you're creating. But if two accounts were created on the same block, the account creation would revert for the second, leading to a poor user experience.
This function allows our peripheral
Onboarding
contract (as well as a UI for just creating an account token without onboarding) to create an account with an incrementing ID with no risk of reversion. Open to renaming to something other thanmintNext
.