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

fix: reinstate economy boot using minInitialPoolLiquidity=0 #5441

Merged
merged 8 commits into from
May 26, 2022

Conversation

dckc
Copy link
Member

@dckc dckc commented May 26, 2022

refs: #5375
obsoletes: #5428

Description

  • add economy bootstrap to CI (by way of make scenario2-run-chain-to-halt in cosmic-swingset)
  • set minInitialPoolLiquidity to 0 in "economy" bootstrap
  • when adding an issuer from an IBC denom (publishInterchainAssetFromBank), add it to the AMM, assuming minInitialPoolLiquidity is 0
  • fix(AMM): await zcf.saveIssuer() (drive-by, found during testing)

Security Considerations

Users of "economy" bootstrap must manage the risk of spam AMM pools.

Documentation Considerations

can't think of any

Testing Considerations

restoring test-gov-collateral.js to working order was a bit challenging; the benefactor.makePool() code is now dead code, but I left a TODO to test that path, so I'm inclined to keep it around.

Comment on lines +45 to +46
async mint => {
await zcf.saveIssuer(secondaryIssuer, keyword);
Copy link
Member

Choose a reason for hiding this comment

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

linking #5407 for motivation

@@ -415,7 +425,7 @@ test('Benefactor can add to reserve', async t => {
await s.startDevNet();
await s.provisionMembers();
await s.startRunPreview();
await s.benefactor.makePool(2000n, 1000n);
// await s.benefactor.makePool(2000n, 1000n);
Copy link
Member

Choose a reason for hiding this comment

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

please delete dead code

Copy link
Member Author

Choose a reason for hiding this comment

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

justification above doesn't suffice?

      // This isn't used now that we make the pool from a denom
      // in publishInterchainAssetFromBank in addAssetToVault.js
      // But it should still work. TODO: Perhaps we should test both ways?
      // i.e. from a board ID as well?

basically: this design has flip-flopped rapidly recently. I'd like to keep it there as a hedge.

dckc added 2 commits May 26, 2022 13:33
 - restore support for $INTERCHAIN_DENOM
 - allow passing env explicitly with process.env as a default
 - fix: minInitialPoolLiquidity goes inside options
 - addPool in publishInterchainAssetFromBank in proposals/addAssetToVault.js
@dckc dckc added the automerge:no-update (expert!) Automatically merge without updates label May 26, 2022
@mergify mergify bot merged commit a98368e into master May 26, 2022
@mergify mergify bot deleted the dc-ibc-pool-min branch May 26, 2022 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants