-
Notifications
You must be signed in to change notification settings - Fork 601
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
SIP-182 Wrapper Factory #1489
SIP-182 Wrapper Factory #1489
Conversation
stubbed currently: * totalIssuedSynths calculation * distributeFees tests to come
pretty much the only remaining thing now is DebtCache related
After discussion it has been decided that the best way to manage debt exclusions for Wrappers is to report them directly to the debtcache, and then the debtcache can calculate the actual excluded debt values and produce result. there is currently one failing test that I don't think I touched (and if I did its a really wierd error)
contracts/SystemSettings.sol
Outdated
function setEtherWrapperMaxETH(uint _maxETH) external onlyOwner { | ||
flexibleStorage().setUIntValue(SETTING_CONTRACT_NAME, SETTING_ETHER_WRAPPER_MAX_ETH, _maxETH); | ||
emit EtherWrapperMaxETHUpdated(_maxETH); | ||
function setWrapperMaxTokenAmount(address _wrapper, uint _maxTokenAmount) external onlyOwnerOrSelf { |
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.
I'd say it would be better to move these into an internal function _setWrapperMaxTokenAmount
here for access control than the modifier.
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.
yea the problem is SystemSettings
is right on the edge of the size limit for optimism contracts... if more internal functions are added could be a problem. Thats the main reason why it is the way it is right now.
contracts/Wrapper.sol
Outdated
|
||
bytes32 internal constant sUSD = "sUSD"; | ||
bytes32 internal constant ETH = "ETH"; | ||
bytes32 internal constant SNX = "SNX"; |
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.
SNX constant is unused
contracts/WrapperFactory.sol
Outdated
wrapper.rebuildCache(); | ||
|
||
// Register it so that MultiCollateralSynth knows to trust it | ||
flexibleStorage().setUIntValue(WRAPPER_FACTORY_CONTRACT_NAME, bytes32(uint(address(wrapper))), WRAPPER_VERSION); |
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.
Hmmm - not sure the versioning is that important here - why not use keccak
to hash the address of the wrapper as the key and then use the address as the value?
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.
its not important. all that matters is to put a non-zero value in the storage slot, and the version is more potential use than just putting 1
. Who knows! maybe it will be helpful later on.
using the address as the value would not be very helpful since the usage of the address
in the key does not glean any additional information that isWrapper
didn't have before.
contracts/SystemSettings.sol
Outdated
flexibleStorage().setUIntValue(SETTING_CONTRACT_NAME, SETTING_ETHER_WRAPPER_BURN_FEE_RATE, _rate); | ||
emit EtherWrapperBurnFeeRateUpdated(_rate); | ||
modifier onlyOwnerOrSelf { | ||
require(msg.sender == owner || msg.sender == address(this), "Only the contract owner may perform this action"); |
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.
if you stick with using a modifier and you use it more than once in a contract, it's better for contract size if you put the logic in a private function and call that in the modifier (see Owned._onlyOwner()
)
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 this because the modifier simply inlines code on the beginning of a function? interesting.
contracts/SystemSettings.sol
Outdated
uint _mintFeeRate, | ||
uint _burnFeeRate | ||
) external onlyOwner { | ||
this.setWrapperMaxTokenAmount(_wrapper, _maxTokenAmount); |
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.
[np] while use this
instead of making the other functions public
?
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.
good idea
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.
we might go over the contract limit with the SIP 135, and public functions cost more gas to deploy... and I don't think you can use public functions with interfaces
contracts/WrapperFactory.sol
Outdated
|
||
// Returns sum of totalIssuedSynths for all wrappers deployed by this contract | ||
function totalIssuedSynths() external view returns (uint) { | ||
return 0; // stub |
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.
missed this?
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.
yes
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.
Overall, PR is looking good. Mostly I think the naming could be made better - as for the logic, tests are passing, so it's looking good. Nice work!
contracts/SystemSettings.sol
Outdated
// The maximum amount of ETH held by the EtherWrapper. | ||
function etherWrapperMaxETH() external view returns (uint) { | ||
return getEtherWrapperMaxETH(); | ||
// SIP 112: Wrappr |
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.
SIP # here should be updated
} | ||
|
||
function _setTargetSynthIssued(uint _targetSynthIssued) internal { | ||
debtCache().recordExcludedDebtChange(currencyKey, int256(_targetSynthIssued) - int256(targetSynthIssued)); |
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.
Target synth is a good choice of name, I like it.
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.
so does that mean token
should be originToken
?
contracts/Wrapper.sol
Outdated
} | ||
|
||
function _burn(uint amount) internal { | ||
uint excessAmount = getReserves() - (targetSynthIssued - amount); |
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.
What does excess amount refer to? Excess of what? Excess capacity?
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.
this function is designed to take any reserves which are unaccounted for and send them to the sUSD fees (that way if someone sends WETH to the contract or something its not locked away forever, and it won't cause unaccounted issuance). Therefore, excessAmount
is the amount of unaccounted reserves that haven't been issued, for which sUSD fees can be taken. Would unaccountedReserves
be a better name?
.filter(l => !!l) | ||
.find(({ name }) => name === 'Issued'), | ||
|
||
describe('mint after transfer without mint', () => { |
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.
Uh, what does this test case do? I don't see any transfers / what does without mint refer to?
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.
there is an edge case where a user could mess up the DebtCache
by using ERC20.transfer
with the target to send funds to the wrapper, and effectively reduce the debt of the system without issuing any synths or fees. I didn't know the implications of this, so this test case was added to make sure proper handling of unaccounted transfers to the wrapper (aka transfer
instead of calling mint
).
describe('transfer without mint', () => { | ||
let preTargetIssuedSynths; | ||
|
||
const extraTransferred = toUnit('2'); |
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.
extraTransferred? Extra of what? I think the naming here can more accurately reflect what's going on
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.
how about unaccountedTransfer
?
@dbeal-eth re: your failing test, the line of code in the test suite test/contracts/SynthetixState.js:166:4 indicates to me that there has been a debt snapshot or some addition to the debt ledger. Maybe your tests have a takeDebtSnapshot() which wasn't surrounded by a |
"how do I represent MultiCollateralSynth change in releases file? by individual synths, or MultiCollateralSynth? just left it empty for right now." See the EtherWrapper PR. You redeploy each individual synth, eg. |
Hi from Optimism! I was asked to take a look for regenesis changes that would affect this PR. To do so, I created a checklist based on our changeset page. In which I've put into this gist, to avoid undue clutter here. Please review that checklist, in particular any thing I did not check, or comments I've prefixed with If you have any questions, feel free to ask here, and also ping/add |
Instead of asking you to review the whole gist that I linked to in the previous message, there are two open items I'd appreciate your help in confirming:
|
No, there are no contracts created using that opcode.
We have a process for verifying contracts and after the kovan release, all contracts remained verified. However, the new version of the Wrapper may not have this verification after it is deployed later this week, so we will need to review and ensure all wrappers craeted by the factory are verified. |
OK, SGTM! |
* use safeTransferFrom * use `issuanceActive` modifier when issuing with `mint` or `burn` * fix potential case of burning more than expected synth * check that fees actually exist before sending fees to WrapperFactory * require that only one of `wrapperMintFeeRate` or `wrapperBurnFeeRate` can be negative at a time
also fix releases file with Synths.*
…/synthetix into abstract-eth-wrapper
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.
LGTM
|
||
require(newExcludedDebt >= 0, "Excluded debt cannot become negative"); | ||
|
||
_excludedIssuedDebt[currencyKey] = uint(newExcludedDebt); |
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.
@dbeal-eth Do we want to update the debt cache cachedDebt
value as well?
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.
after discussion this will be reviewed and replaced if necessary before mainnet later this week
return (excludedDebt, isInvalid); | ||
} | ||
for (uint i = 0; i < currencyKeys.length; i++) { | ||
excludedDebt = excludedDebt.add(_excludedIssuedDebt[currencyKeys[i]].multiplyDecimalRound(rates[i])); |
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.
@dbeal-eth Why not track and update the delta for _excludedDebt
as a single value and then you don't need to recompute the sum fo all _excludedIssuedDebt
here when taking snapshot.
I don't see where else the individual _excludedIssuedDebt[keys]
are used atm. I think it is good to have a granular breakdown of each excluded issued debt but you can optimise this here with rolling up the values as a single excludedDebt
value.
This is what is currently used to store the excluded debt value _cachedSynthDebt[EXCLUDED_DEBT_KEY] = excludedDebt
https://github.com/Synthetixio/synthetix/blob/master/contracts/DebtCache.sol#L38-L39
|
||
require(newExcludedDebt >= 0, "Excluded debt cannot become negative"); | ||
|
||
_excludedIssuedDebt[currencyKey] = uint(newExcludedDebt); |
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.
As comment above, consider updating a single value to roll up excludedDebt
saves iterating over a mapping of all excluded debts when taking snapshot
* Futures closure fee (#1417) * kovan-ovm-futures (refactored) (#1400) * Add confirmation and liquidation booleans to futures position data. (#1424) * add missing chainlink feeds to kovan-ovm-futures * re-run deployment with correct network set * Add global futures settings to data contract. (#1431) * Futures markets respect fee reclamation. (#1436) * Futures markets respect fee reclamation. * Check necessity of reclamation on reclaim value rather than number of entries settled. * Fix merge history for futures-implementation (#1445) * Alioth release (#1263) * Remove bridge migrator (#1257) * Skips multicollateral prod tests if max debt has been reached (#1259) * SIP-136: MultiCollateral/CollateralEth debt is not excluded correctly from debt calculations (#1243) * SIP-112: EtherWrapper (#1178) * Deploy 2.45 to kovan (#1260) * Prepublish step * 2.45.0-alpha * set mint fee and burn fee per sccp 100 Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> Co-authored-by: Jackson C <jackosmacko@gmail.com> * Introducing new integration tests in CI (#1287) * Introducing integration tests in CI * Improved integration tests in CI and removed redundant prod tests * Bugfix on integration test task * More verbose on deployer error * Refactoring of Exchanger.sol to reduce size on OVM (#1291) * Minor fix for integration tests (#1295) * Clean state on dual integration tests plus slightly better exchange tests * Minor fix to integration tests * Disable some exchanging integration tests for now * Extra prod tests (#1299) * Add issuance prod tests * Add erc20 behavior * Tweaks on incoming integration tests * Minor fix to integrationt ests Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Audit feedback and spring cleaning (#1300) * Port more integration tests (#1288) * Basic integration tests for L2 deposits * Basic integration tests for L2 deposits * Polish deposit integration tests * Progress porting withdrawal integration tests * Working withdrawals in new integration tests * Using watcher tool more * Unify action and actionTo in integration tests * Progress porting integration tests * Remove comment * Implemented exchanges in new integration tests * Bugfix in integrationt est task * Basic forking in integration tests * Include fork tests in CI * Minor fixes for prod tests * port migrateEscrow test to integration dual * Fix CI * Fix CI Co-authored-by: Leonardo Massazza <lmassazza+github@gmail.com> Co-authored-by: Yannis <i.stamelakos@gmail.com> * Better way to get SNX in integration tests (#1303) * Better way to get SNX * 2192 * Introduces forking via integration tests (#1307) * Introduces forking with integration tests * Compile and deploy on fork tests * Fix old ovm prod tests * Replace web3 with ethers in deployment (#1271) * add wrapper object to hold web3 and ethers * Replace web3-utils with ethers * - replace web3.utils with ethers counterpart - create account with ethers instead of web3 * corrections to failed tests * propagate Deploy changes fix (test:publish green) * document the provider addition to Deployer * document the provider addition to Deployer * fix require * fix require * Clean install * Update to develop Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Updates optimism dependencies (#1308) * Updated optimism deps * Update usage of dep in code * Add hardhat-ethers dep required by smock * Add await in unit test * Removed @gas-skip (#1309) * Updated optimism deps * Update usage of dep in code * Add hardhat-ethers dep required by smock * Removed @gas-skip * Add await in unit test * Disables prod tests from CI (#1311) * Reordering the deploy script for sanity (#1304) * Add integration tests for settle and claim (#1310) * Support settlements in exchanging behavior * Testing claims in integration tests * Address PR feedback, increase timeouts, improve test for forking * Bigger tolerance for debt comparison * Approve bridge tokens during bootstrap * Add SynthsUSD integration (L1 and L2) tests (#1312) * Add support to SynthsUSD in L1 and L2 integration tests * Add support to SynthsUSD in L1 and L2 integration tests * style fixes Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Port migrateEscrow and depositAndMigrateEscrow tests (#1306) * ported. Some tests still failing * wip * migrateEscrow test ported * accept multiples hashes in watchers * Reduce migrateEscrow dual test to dual scope * ported depositAndMigrateEscrow test * update test wording Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Migration for bulk staking reward upgrade (#1301) * Deleted everything related to prod tests (#1315) * Adds ether wrapper integration tests (#1319) * Basic integration tests for ether wrapper * Fix timeout in integration tests * Abstract eth wrapper behavior in integration tests * Support ether wrapper integration tests in mainnet forks * Pin ops to a particular commit that is known to work in CI * Processed Leo's feeedback on PR * Enables separate folder compilation for integration tests (#1322) * Enables separate folder compilation for integration tests * Clean install * Merge all integration test tasks into one (#1323) * Remove web3 from the deploy script test (#1328) * Progress removing web3 from test for deploy script * Progress removing web3 from test for deploy script * Almost done removing web3 from deploy tests * Add optimism scripts to run Optmism via hardhat (#1324) * Add optimism scripts to npm * fix variable naming * Add harhat task to build and start ops * remove ops scripts from packages.json * - check status to run the right steps - order jobs - cleanup messages * Update circleCI * change docker command to test on circleCI * keep chain atached and add detach option * Tweak CI for integration tests * Keep ops start task open Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Unpins the usage of the Optimism ops tool and starts managing L2 gas (#1331) * Removes web3 from nominate script (#1332) * Progress removing web3 from nominate script * Applies nominate test to L2 * Ported nominate script to ethers * Fix * Removed web3 from settle script (#1334) * Removed web3 from extract-staking-balances (#1335) * Remove web3 * Remove wallet (only read from contracts) * Removes web3 from the owner script (#1336) * Using owner script in integration tests * Removed web3 from owner script * Manually setting isContract in owner script * Add caching of docker layers on integration tests job (#1333) * remove docker prune from ops tasks * add cache for optimism build on integration-tests job * add a check to execute deployments tests only if a deployment.json file changed (#1339) * Removes web3 from the purge script (#1341) * Support forks in purge-synths script * Extract performTransactionalStep version for ethers * Rename function to deprecate * Minor fix * Remove web3 from remove-fee-periods (#1342) * wip commit * wip commit * wip * wip * Remove log line from script * remove empty comment * Add gas reporting job parallelization on CI (#1305) * add task for merging gas reports files on CI * add parallelized gas reports to unit tests * update codechecks unit-test-gas-report name * remove optimizer flag from unit tests * remove test:gas script from package.json * remove web3 references (#1344) * SIP-150 Fix excluded debt calculation for partial snapshots. (#1340) * SIP-145 Emit the proper cached debt number when debt snapshots are taken. (#1325) * remove web3 from migrate-binary-option-markets (#1345) * Fixing nominate to work locally and ethers fix (#1354) * Introduces fast forwarding in L2 integration tests (#1343) * Basic fast forwarding in L2 integration tests * Replaced 'ignore' utils with fast forwarding in integration tests * Clean install * Undo changes to package lock file * Pin ops version * Avoid redundant heartbeats * Fix and improve integration tests * Add comment to test * Update ops image * Clear ops cache in CI * Update watchers dep * Hotfix for Optimism watchers * Clean install * Update to hardhat 2.3.x * Clear ops tool cache * Unpin ops * Update ops cache * Attach ops output * Debugging Optimism messenger watchers * Listen to interactions with messenger on blocks * Keep withdraw tests open * Implementing a completely patched ops watcher * Tidy ups * remove only in tests * Disabled ops cache * Restored ops caching * Quiet * Bugfix * Debug optimism in l2 standalone integration tests * Cleanups * Adds integration tests for opening and closing a loan (#1330) * Resolves conflict * Updates ignore waiting period to new pattern * Deploy EmptyEtherWrapper (#1349) * EmptyEtherWrapper * deploy EmptyEtherWrapper * ignore EmptyEtherWrapper for coverage * Use simple synths in local-ovm * Bugfix on ops tool task Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * remove web3 from checkAggregatorPrices (#1353) Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Unify wallet creation and ensure .address is always present (#1357) Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Support for testnet forking (#1359) * Allow private key overrides in local and fork mode (#1360) * Fix for SupplySchedule - must use the ERC20 proxy (#1338) * Adding new synth suspension reason for index rebalancing and using testnet checksum address * SCCP-98 DEFI rebalance (#1364) * Prepublish step * 2.45.3 * Refresh ops cache (#1368) * Upgrading etherscan links to optimistic explorer when required (#1369) * Ethers overrides gasLimit, not gas (#1366) * Adds documentation for integration tests (#1367) * Add documentation for integration tests * Update integration tests README * Updating releases for Alnitak (#1363) * Remove web3 from persist-tokens. (#1352) * remove from persist-tokens. Missing one command * WIP Commit (to stage the branch). Need to replace SetContenthash with our own implementation * wip * Remove web3 dependency (setContenthash doesn't work) * Remove commented out code and useless interaction Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> * Incorporate synthetix-cli interactive-ui as a `npx hardhat interact` task (#1365) * add interactive ui hardhat task * update pacakge-lock with a clean install * Minor tidy ups * Remove redundant utils Co-authored-by: Alejandro <palebluedot@gmail.com> * Adding solidity output to deploy script (#1313) * Simplify conditional logic of generate solidity (#1378) * Alnitak release kovan (#1379) * Prepublish step * 2.46.0-alpha * Adding EtherWrapper for L2 to deploy empty and SystemSettings for Etherwrapper settings (#1380) * Alnitak release kovan optimism (#1381) * Prepublish step * 2.46.0-alpha-ovm * Fixing local dev to properly only deploy WETH the first time (#1384) * Removing 145 and 150 from Alnitak (#1386) * Fix fork-tests to work and surface errors correctly (#1385) * OVM gas limit fixes (#1382) * Mainnet deploy of Alnitak contracts and migration (#1383) * Prepublish step * 2.46.0 * Alnitak release optimism (#1390) * Prepublish step * 2.46.0-ovm * Fixes integration tests on CI (#1392) * Makes l2 standalone integration tests pass * Refresh ops tool cache in CI * Possible fix to hardhat ops task * Undo bad fix, but with small tidy up * Try to build ops image manually * Update CI build * Small bugfix on ci * Another attempt at building ops tool on ci * Undo all changes to CI * Disabled ops tool in CI * Disable ops tool build * Showing a different name while integration tests are simplified * Pins a newer ops tool version * Re-enable ops-tool in CI (#1395) * use docker 20.10.6 in CI * duh... * fix bootstrap.js issue on dual Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> Co-authored-by: Jackson C <jackosmacko@gmail.com> Co-authored-by: Justin J. Moses <justinjmoses@gmail.com> Co-authored-by: justin j. moses <justin@synthetix.io> Co-authored-by: Yannis <i.stamelakos@gmail.com> Co-authored-by: Leonardo Massazza <lmassazza+github@gmail.com> Co-authored-by: Matías Lescano <mjlescano@users.noreply.github.com> Co-authored-by: Anton Jurisevic <zyzek@users.noreply.github.com> Co-authored-by: David Goldberg <gberg1@users.noreply.github.com> * Futures order slippage (#1446) * add maxSlippage to submitOrder This will be used to calculate an upper/lower bound for the price upon an order being confirmed. * add slippage to calls * failing test for confirmOrder slippage * passing test for confirmOrder slippage * fix some tests * * fix/simplify slippage calc * fix tests * fix test * convert slippage param to price bounds (min, max) * fix tests * test order min/max price in data contract * canConfirmOrder respects max market size constraints + error management + order size/status (#1430) * Futures: Market deployment and management scripts (#1440) * add futures-markets.json to deploy configs * load futures market settings from config * remove FUTURES_ASSETS config var * fix * add futures-markets.json to deploy configs * load futures market settings from config * remove FUTURES_ASSETS config var * fix * wip * wip2 * restore * * modify publish script test to work with OVM * add test for futures markets being added * fix: use only hardhat for L2 tests * publish script working for local-ovm * add optimism folder to eslintignore file (#1406) * restore publish script to original l1-only design My original design was too complex for not much benefit, since most of the tests were skipped anyways. * add l2 publish script test, as separate block below This is a much cleaner diff that'll enable me to get this work out quicker. * lint * remove publish test block I'll reintegrate this in another PR. Co-authored-by: Matías Lescano <mjlescano@users.noreply.github.com> * Richer Futures position and margin events (#1456) * Futures documentation (#1460) * Dead code cleanup. (#1462) * Position ID, PositionOpened, PositionClosed added (#1461) * Position ID, PositionOpened, PositionClosed added * removed useless positionId variable * PR fixes * removing PositionOpened & positionClosed * Adding new status to futures market interface * test debug * tests added * Futures spot trades (#1477) * Futures position details (#1479) * Futures accessible margin (#1484) * emit sizeDelta with PositionModified event (#1485) * emit sizeDelta with PositionModified event * fix * sizeDelta -> tradeSize This is clearer for liquidation flows * Deploy kovan-ovm-futures v0.4.0 (Alpha) (#1487) * remove price update constraint from order flow * prepare-deploy * add FuturesMarketSettings to releases * deploy kovan-ovm-futures v0.2.0 * fixes * unignore deployment.json * add FuturesMarketSettings to releases * prepare-deploy * deploy kovan-ovm-futures v0.3.0 * Futures position details (#1479) * Futures accessible margin (#1484) * emit sizeDelta with PositionModified event * emit sizeDelta with PositionModified event (#1485) * emit sizeDelta with PositionModified event * fix * sizeDelta -> tradeSize This is clearer for liquidation flows * deploy kovan-ovm-futures v0.4.0 * test that npmignore will include deployment.json NPM ignores files listed in gitignore which is a problem. https://stackoverflow.com/questions/24942161/does-npm-ignore-files-listed-in-gitignore Co-authored-by: Anton Jurisevic <zyzek@users.noreply.github.com> * Fix futures position id management + tests. (#1492) * fix slither crashing in CI and local (#1519) fix slither crashing in ci and local #1519 * Adding gnosis safe multisend functionality to owner (#1513) * Adding safe functionality from the gnosis SDK to the owner command * Updating nominate command to use owner actions when not the owner * SIP-135 Cleanup (#1522) * Prepping for the Sargas (2.50) release to OVM (#1526) * Sargas to be OVM only with 135, 142 and 174 * Adding missing SIP-140 from list * Fixing settle for ethers (#1509) * Reduce gas usage for opening loans (#1529) * Print deploy param object value (#1534) * add slither code check github integration (#1523) * Refactor exceedsDebtLimit check (#1539) * Removing unneeded kovan contracts (#1538) * ci: lavamoat integration (#1517) * CI: add codeql (#1544) * Adds block tag parameter to interact task (#1528) (#1536) * ci: slither github actions improvements (#1543) * Removing integrationProxy from Proxyable (#1521) * up maxMarketValue * Fix prepare deploy (#1548) * Document debt cache (#1533) * * conceptually document this function * iterate on a simplified construction * rewrite doc for current version, sans changes to logic * document debt cache * Updating packages for audits and temporarily disabling the audit check (#1553) * redeploy again * ci: fail audit on critical severity (#1557) Signed-off-by: Jakub Mucha <jakub.mucha@icloud.com> * ci: update docker containers to node@14.18 (#1558) * chore: name entry added to lockfile in recent npm version * ci: update docker containers to node@14.18 * Futures: merge develop (#1547) * Update proportional skew funding calculation to use maxMarketValue an… (#1556) * Update proportional skew funding calculation to use maxMarketValue and also cap maxMarketValue * update funding rate skew tests and IFuturesMarket interface * Futures min skew scale (#1561) * use minSkewScale to control starting funding rate * enable skipped tests * increase testnet minSkewScale * add test for minSkewScale regime transition Co-authored-by: Arthur Deygin <29574203+artdgn@users.noreply.github.com> * add verified details for some (not all) contracts * only run if CollateralShort exists * * Move "sources" into "sips" * Add additional sources to SIP-80 * fix: disable WETH deployment on L2 * deploy kovan-ovm-futures v0.5.0 * add whitelist for transfering externstateToken (#1565) * add whitelist for transfering externstateToken * update require statement * enable limited transfers on kovan-ovm-futures * introduce LimitedTransferSynth, a clone of Synth with limited transfers according to whitelist * polymorphic approach to deploying LimitedTransferSynth Co-authored-by: liamzebedee <liamzebedee@yahoo.com.au> * prepare-deploy kovan-ovm-futures v0.5.1 * prepare-deploy 0.5.2 We need to replace Issuer as the synths are being added there, not in Synthetix * prepare-deploy 0.5.3 hack: deploy SynthRedeemer so we can get out-the-door * deploy 0.5.3 * revert changes * Deploy kovan-ovm-futures v0.5.0 (#1566) * add verified details for some (not all) contracts * only run if CollateralShort exists * * Move "sources" into "sips" * Add additional sources to SIP-80 * fix: disable WETH deployment on L2 * deploy kovan-ovm-futures v0.5.0 * prepare-deploy kovan-ovm-futures v0.5.1 * prepare-deploy 0.5.2 We need to replace Issuer as the synths are being added there, not in Synthetix * prepare-deploy 0.5.3 hack: deploy SynthRedeemer so we can get out-the-door * deploy 0.5.3 * revert changes * add missing deployments to config.json so verify picks them up * verified * prepare-deploy 0.5.4 * deploy SynthsETH and SynthsUSD * During deployment, ensure deprecated synths from Wezen have cache updated (#1564) * Update configure loans interactionDelay (#1568) * update deployer to support EIP-1559 (#1504) * update deployer to support EIP-1559 updates `--gas-price` to be replaced with `--max-fee-per-gas` which specifies the maximum base fee paid on a transaction. Additionally, deployer may also specify `--max-priority-fee-per-gas` to specify a mining tip (default: 1 gwei) If the network does not support 1559, the `gasPrice` is automatically determined by ethers. For EIP-1559 support, ethers.js needed to be upgraded to 5.4.6 Co-authored-by: jj <jj@og.snxdao.io> * futures listen to system and synth suspensions (#1530) * Deploy kovan-ovm-futures v0.6.0 (#1570) * log error here for better info * upgrade deployer for OVM 2.0 * fix: update gas price * prepare-deploy 0.6.0 * fix: rm WETH deploy * fix url's to etherscan * SIP-187 fix partial synth updates and debt cache updates (#1551) * fix partial synth updates and debt cache updates * Remove require check that cachedSum < Debt as excluded Debt can cause this to fail. Update calc of delta in new synths changed. * * revert max market value to higher amount * fix network key in deployment.json * remove gasPrice overrides from simulate-deploy (#1573) * Exchange rates circuit breaker refactor (#1540) * SIP-182 Wrapper Factory (#1489) Co-authored-by: Lecky <leckylao@gmail.com> Co-authored-by: Mark Barrasso <4982406+barrasso@users.noreply.github.com> * SIP-182 Audit feedback - allowance in constructor (#1584) * Fix param spam during deployment (#1587) * add missing data in position modified events (#1580) * add expalantion about releases.json in readme (#1569) Co-authored-by: Liam Zebedee <liamzebedee@yahoo.com.au> Co-authored-by: jj <jj@og.snxdao.io> * ci: use node 16 (LTS) (#1588) * ci: use node@16 * ci: use new cache key * chore: "name" missing in package-lock.json * ci: generated new config * chore: use node@16 by default Signed-off-by: Jakub Mucha <jakub.mucha@icloud.com> * Removing inverse synths (#1592) * Ensuring SCCP-139 feeds are persisted to future ExchangeRates contracts (#1593) * Futures rename parameters (#1595) * Deploy kovan-ovm-futures v0.7.0 (#1589) * log error here for better info * upgrade deployer for OVM 2.0 * fix: load source for LegacyOwned LegacyOwned is contained within legacy/ * remove ovm-specific logic around compilerversion metadata Code is now compiled using the regular solc. * update gas price again * prepare-deploy * deploy kovan-ovm-futures v0.7.0 * rm WrapperFactory from config I copied these changes hackily across from develop. This snuck through * contracts verified * deploy kovan-ovm-futures v0.7.0 again For some reason, LimitedTransferSynth wasn't deployed for SynthsUSD the first time * update ops node * use the optimismCommit in task-ops.js as a cache key * Skip shorts integration tests when cannot open loans (#1597) * Add default private key for local-ovm deploys (#1455) * add default private key for local-ovm deploys * fix: add useOvm * Update ops node commit to working version in `develop` * SIP-120: TWAP Exchange Function (#1127) * Adding 6 more potential releases * Helper script to distribute SNX/sUSD to accounts on kovan-futures-ovm (#1554) * commit empty deployment.json for local-ovm so resolve will work * move local-ovm network down * iterate on local helper for bootstrapping * working script to bootstrap local l2 account (aka snx-brrr) * local script now supports specifying provider/snx-network/owner account * use ensureBalance to get testnet SNX/sUSD for accounts * refactoring: balances script * log error here for better info * upgrade deployer for OVM 2.0 * fix: update gas price * prepare-deploy 0.6.0 * fix: rm WETH deploy * update local dist script with whitelist * revert * rename file to match task * Futures liquidation fee update (#1594) * adding market debt explanation comments * Fixing owner script to not submit multiple accept for dupe contracts * Upgrading to OVM 2.0 (or the destruction of useless code) (#1598) * Minor fixes for release history * SIP-167 Introduces an L2 governance bridge (#1402) * Removing mistake * Fixes from menkalinan (#1608) * Owner batch fixing * Adding L2 owner * Update OVM bytecode (#1613) * Move debt snapshot to beforeEach block (#1616) * Fix duplicate fee reporting on WrapperFactory (#1617) * SIP-194 Fix Liquidations on L2 (#1621) * Fix broken unit test (#1622) * Futures next price mechanism (#1609) * Futures remove closure fee and rounding (#1610) * reduce storage variables sizes for gas savings (#1614) * Ensuring job-compile size check matches build command (#1628) * refactor views into mixin to reduce clutter (#1615) * rename circuit breaker contracts (#1629) * Adding exchange gas usage output * Adding gas output to synth issue, burn and claim in int tests * SIP-188: Add sETHBTC synth (#1618) * add tests for debt cache when markets * SIP-195: L2 CollateralEth Loans (#1632) * Sip 196 remove internal oracle (#1636) * Fix contracts compiling after merge * Remove LimitedTransferSynth * add liquidation tests in integration (#1625) * use new `hardhat-interact` package instead of builtin (#1612) * fix liquidations fork test fail (#1646) * SIP-193 Reduce size for SystemSettings (#1627) * Implement interface funcs for BaseDebtCache, EmptyCollateralManager, IExchanger interface cleanup * Update configuration of loan and system settings (#1637) * SIP-200: Fix FeePool Rewards Distribution (#1650) * migration script helper allows deployment and staging of migration script call (#1652) this should make releases a little easier and make it easier to utilize the migration script functionality of the deployer. * deploy futures to kovan-ovm-futures and update configuration scripts * fix shadowing in empty futures market * fix circuit breaker tests * remove deprecated setLastExchangeRateForSynth * update fund local accounts script * fix atomic exchange circuit breaker tests * fix market settings and manager tests * SIP-184 Dynamic Fees (#1649) * fix status script and fund local accounts * Take debt snapshot before funding local accounts (#1657) * fix aggregators usage in futures, setup fixes * fix futures market test setup * fix futures market data tests * fix next price test setup * fix system settings tests * fix integration tests failing to setup markets * fix market debt calculation during setup * Deploy SNX/ETH and sUSD/DAI staking rewards on OVM (#1653) * remove old RewardEscrow from deployment.json for ovm environments (#1658) * Update kovan-ovm/feeds.json for OCR (#1659) * verify deployment * fix market settings tests * fix futures market tests * Update feeds.json for kovan ovm with new OCR proxies (#1664) * Optimism Forking with Hardhat (#1656) Co-authored-by: jj <jj@og.snxdao.io> * Remove legacy deployment config from kovan-ovm (#1665) * Upgrade Gnosis SDK (#1655) * SIPS 196, 193, 184 - audit fixes, full version (#1661) * Futures merge 184 (#1672) * fix breaker merge test * fix test lints (#1607) * fix test lints sometimes the dual tests fail due to race condition built into the tests in addition, a better event is now being used for monitoring transaction relay status. Withdrawal tests still don't work because the transaction is not relayed on the L1 side, I'm guessing because the user has to call `finalize` transaction somehow. Not sure who to talk to if we want to get those tests working. * fix lints * uncomment Co-authored-by: Mark Barrasso <4982406+barrasso@users.noreply.github.com> Co-authored-by: jj <jj@og.snxdao.io> * try fix dual itnegration tests * update migration script to also stage nominations (#1670) * update migration script to also stage nominations also the owner can now run migration, instead of the deployer, which is way better for getting stuff done * fix lints * re-add legacy onlyDeploy function for older migrations * add verify step * use performTransactionalStep * add signer * cleanup verify step and additional logging for clarity * another attempt to fix dual integration tests * Revert "another attempt to fix dual integration tests" This reverts commit 64de1c2. * Revert "try fix dual itnegration tests" This reverts commit 8d0f088. * Removing redundant ExchangeRatesWithInversePricing * Ensuring fork tests do compile (#1679) * Futures dynamic fee (#1673) * Sip 185 debt shares (#1601) * Fix the validate deployment CI run (#1680) * Removing the redundant owner param from migrations (#1681) * Collecting Test Metadata in CI (#1683) * SIP-209 Update feeRateForExchange function signature (#1686) * Also fix test-coverage in CI * Updating task-node to properly manage the provider * Futures size reduction (#1682) * address preliminary audit comments (#1691) * Futures pausing (#1692) * Basic npm audit fix (#1693) * Updating mainnet-ovm feeds to new chainlink OCR (#1645) * add moar releases (#1698) * Fixing ABI anomaly for kovan-ovm for exchangeWithVirtual * AIP-202 - Upgrade supply schedule to use target ratio inflation amount (#1700) * Fixing lint * Updating mainnet-ovm feeds to new chainlink OCR (#1645) * add moar releases (#1698) * Polaris release 2.61 to mainnet ovm (#1699) * SIP-199 sSOL to Optimism * SCCP-163 OCR feeds on Optimism * Prepublish step * 2.61.0 * Fixing ABI anomaly for kovan-ovm for exchangeWithVirtual * Fixing ABI anomaly for kovan-ovm for exchangeWithVirtual * 2.61.1 * AIP-202 - Upgrade supply schedule to use target ratio inflation amount (#1700) * Hamal release v2.62 mainnet (#1706) * SIP-202 Target Staking Ratio * Prepublish step * 2.62.0 * Fixing lint * deploy new futuresMarkets with dynamic fees to kovan-ovm-futures * verify * add ovm etherscan key support (#1703) * address minor audit issues * deploy systemStatus * release exchangeRates * dual integration test fix attempt * Futures single market pausing (#1711) * Futures support multiple markets for same asset (#1713) * Futures volume source fee methods (#1714) * update releases json * revert unrelated contract changes * fix fork tests (#1717) * Debtcache import excluded debt entries (#1716) * SIP-165 Debt Orcale (#1694) Impl for 165 From a code perspective, this entails: FeePool and Issuer are changed to use the chainlink oracle instead of DebtCache and SynthetixDebtShare directly FeePool now closes its fee period across networks (using optimism relay call) to allow for synchronized sharing of close parameters between networks For testing and initial deployment, a dummy oracles SingleNetworkAggregatorDebtRatio and SingleNetworkAggregatorIssuedSynths are utilized to retrieve debt values for this network, meaning most unit tests can work exactly the same as before. Notes: The SC has indicated that inflation should be divided evenly between networks based on amount of debt shares on each network, so this has been implemented. Also, fees will remain on the network they originate from for the time being Dual Integration test was added to verify fee pool closure Tests were removed from DebtCache because the functionality is no longer used within the system, but the actual code from solidity was not removed because there is no need to include DebtCache in an update. Doing so would require more migration complexity and it would be better if we could avoid that, so no changes have been made to DebtCache for the time being. The release process for this SIP is 2 steps: First, we will release as usual with the included SingleNetworkAggregators, which will preserve current functionality while enabling for us to start reading from an oracle interface for debt info Second, we will use the pdao to change the AddressResolver setting for the two aggregators to be the chainlink provided ones, which will effectively complete the debt synthethsis and enable synth fungibility * diphda part 1 * diphda part 2 * diphda optimism part 1 * diphda ovm part 2 Co-authored-by: Anton Jurisevic <zyzek@users.noreply.github.com> Co-authored-by: Liam Zebedeee <liamzebedee@yahoo.com.au> Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> Co-authored-by: Jackson C <jackosmacko@gmail.com> Co-authored-by: Justin J. Moses <justinjmoses@gmail.com> Co-authored-by: justin j. moses <justin@synthetix.io> Co-authored-by: Yannis <i.stamelakos@gmail.com> Co-authored-by: Leonardo Massazza <lmassazza+github@gmail.com> Co-authored-by: Matías Lescano <mjlescano@users.noreply.github.com> Co-authored-by: David Goldberg <gberg1@users.noreply.github.com> Co-authored-by: Clément BALESTRAT <clement.balestrat@gmail.com> Co-authored-by: Mark Barrasso <4982406+barrasso@users.noreply.github.com> Co-authored-by: Jakub Mucha <jakub.mucha@icloud.com> Co-authored-by: jj <jj@og.snxdao.io> Co-authored-by: dbeal <git@dbeal.dev> Co-authored-by: Lecky <leckylao@gmail.com> Co-authored-by: Brett Sun <qisheng.brett.sun@gmail.com> Co-authored-by: Joey <5688912+bachstatter@users.noreply.github.com> Co-authored-by: Noah Litvin <335975+noahlitvin@users.noreply.github.com> Co-authored-by: dbeal <git@danb.email>
Implementation features a new contract,
WrapperFactory
which provisionsWrapper
contracts (following API of originalEtherWrapper
) upon call tocreateWrapper
Note tests are 99% working/done despite CI failure.
Notes:
EtherWrapper
implementationDebtCache
includes changes to accept issuance information from "debt issuers". Currently only wrapper uses this, but in the future (v3) it could be rethought as a mechanism for tracking issuance from any kind of debt issuer and excluding from the snx debt. This also changes the deployment process forDebtCache
a little bit, since issued debt will need to be re-reported to the DebtCache on redeploy. Planning on implementing a possible util function for this.EtherWrapper
has been removed. There is a branch where it has not been removed if it should be re-addedQuestions:
MultiCollateralSynth
change in releases file? by individual synths, orMultiCollateralSynth
? just left it empty for right now.