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

Rewrite loadgen to use a dedicated asset #71

Merged
merged 12 commits into from
Mar 24, 2022
Merged

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Mar 21, 2022

Closes #69
Refs Agoric/agoric-sdk#4634

Best reviewed commit-by-commit

Agoric/agoric-sdk#4541 removed the AMM, VaultFactory and demo assets from the default bootstrap. While the demo config does have the AMM and VaultFactory enabled, it doesn't contain any assets the loadgen could use for its tasks.

Agoric/agoric-sdk#4641 added the ability for the loadgen to bless an asset as usable with vaults. This PR mainly updates the loadgen to create its own asset, create an AMM pool, fund it with that asset and some RUN, and bless the asset for usage with the VaultFactory.

This PR ends up as a complete rewrite of the task prepare steps, and make major changes to the tasks themselves. Some noteworthy changes:

  • Remove sending distributed objects back to the deploy script in the result of tasks (Amount were previously shared)
  • Fix display of assets to respect decimal places
  • Pipeline operations as much as possible

I have tested this change against a range of previous revisions to insure backwards compatibility. If the vaultFactoryCreator isn't available, the prepare step attempts to fallback to a demo asset (USDC), if available. In the worst case, the vault task will not be functional, but the amm task should always be available with the new dedicated loadgen asset.

e43902478 # grant addVaultType based on addr (03/19/2022)
22356c2b4 # Original base of addVaultType PR (02/22/2022)
492769f2f # Before change removing USDC (02/21/2022)
80f0076d5 # last known good before BLD removal? (01/24/2022)
3c4b2fb71 # After zoeFeePurse removal (12/29/2021)
01a174e88 # Zoe metering (08/17/2021)
e0f816e15 # Know good before metering (08/15/2021)

@mhofman mhofman requested review from warner and dckc March 21, 2022 22:51
@mhofman
Copy link
Member Author

mhofman commented Mar 21, 2022

I have a rebased version with the fixup commits squashed. @warner @dckc, what do you prefer reviewing? A more condensed version, or something closer to the original progress?

@dckc
Copy link
Member

dckc commented Mar 21, 2022

I'll take a look as-is and let you know if I'd rather have something else...

@mhofman mhofman force-pushed the mhofman/use-loadgen-token branch 2 times, most recently from a3d04eb to 5b80462 Compare March 21, 2022 23:02
@mhofman
Copy link
Member Author

mhofman commented Mar 21, 2022

I'll take a look as-is and let you know if I'd rather have something else...

Oops sorry, I thought this was a lot of commits to review one by one.

  • The original version is ad1cd08
  • The re-ordered version of the above with minimal conflict resolution (I didn't manage to layer all of it right): a3d04eb
  • The squashed version (current state of this PR): 5b80462

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I spent a little time looking at all the code in the PR both by-commit and by-file and it (5b80462) looks OK. I'm not entirely confident that I understand the code well enough to maintain it, but I'm willing to risk it.

I wonder about moving some of the agent / task stuff into agoric-sdk tests to catch integration problems earlier.

I did not verify that I can run it. I wonder if @arirubinstein is interested to try it out? I don't suppose that's critical to landing this PR, though.

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

I only did a cursory review, but didn't see any problems.

@mhofman
Copy link
Member Author

mhofman commented Mar 24, 2022

I added a couple commits to handle testnet mode for this, so that the agoric-sdk CI can leverage this as well.

@mhofman mhofman merged commit 495ced1 into main Mar 24, 2022
@mhofman mhofman deleted the mhofman/use-loadgen-token branch March 24, 2022 21:55
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.

Perform loadgen tasks with installed asset
3 participants