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

Figure out how to sequentialize vaults core evals #9841

Closed
mhofman opened this issue Aug 5, 2024 · 11 comments
Closed

Figure out how to sequentialize vaults core evals #9841

mhofman opened this issue Aug 5, 2024 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@mhofman
Copy link
Member

mhofman commented Aug 5, 2024

What is the Problem Being Solved?

Currently multiple core evals scrips in a single core eval action all run in parallel. For vaults we need to have some steps done in sequential order. As part of a chain software upgrade we were able to easily do that because running core proposals there has more features (see #8908 for some related info). Now that we're switching to core eval, we need an equivalent mechnaism

Description of the Design

Either use a single core eval script that manually manages the sequential steps, or leverage the multi message support in the new cosmos x/gov module to submit multiple core eval actions in the same cosmos governance proposal (see #9015 for related info)

Security Considerations

None

Scaling Considerations

None

Test Plan

a3p integration test

Upgrade Considerations

See above

@mhofman mhofman added the enhancement New feature or request label Aug 5, 2024
@aj-agoric aj-agoric assigned aj-agoric and Chris-Hibbert and unassigned aj-agoric Aug 6, 2024
@LuqiPan LuqiPan assigned LuqiPan and unassigned Chris-Hibbert Aug 6, 2024
@mhofman
Copy link
Member Author

mhofman commented Aug 8, 2024

After some quick command line experiments we believe we can generate a cosmos.gov.v1.MsgSubmitProposal (the new cosmos 0.46 gov message type) with multiple agoric.swingset.CoreEvalProposal in its messages, and that the chain will automatically enqueue these separate core-eval actions that would be processed sequentially.

We don't have CLI support to create such a tx, so either we need to modify the agd client, or we need a tool/script to generate the proposal leveraging the existing agd commands. Such a script would likely "generate only" multiple core-eval gov proposals using the legacy command, extract each agoric.swingset.CoreEvalProposal message from the content (using jq), generate a composite draft_proposal.json, and use that to submit a new cosmos.gov.v1.MsgSubmitProposal gov proposal.

We also need to actually test this would be working (as an a3p integration test). We cannot rely on the a3p built-in core eval mechanism, and will need an explicit eval.sh script (as well as generate submissions)

@LuqiPan LuqiPan assigned Chris-Hibbert and unassigned LuqiPan Aug 8, 2024
@Chris-Hibbert
Copy link
Contributor

@dckc suggests using the promise space available in the bootstrap environment that proposals run in to resolve the timing. My requirement is that the vault upgrade shouldn't start until auctions, priceFeeds, and scaledPriceAuthorities have all been restarted. The simple solution to that is to have each of them resolve a promise when they finish, and have vaults wait for Promise.all() of its three prerequisites before upgrading.

Is there a reason that approach wouldn't just work?

@dckc
Copy link
Member

dckc commented Aug 8, 2024

Description of the Design

Either use a single core eval script that manually manages the sequential steps, or leverage the multi message support in the new cosmos x/gov module to submit multiple core eval actions in the same cosmos governance proposal (see #9015 for related info)

we don't need multiple cosmos messages to run multiple scripts.

The existing message CoreEvalProposal accomodates a list of them:

repeated CoreEval evals = 3 [(gogoproto.nullable) = false];

likewise agd tx gov submit-proposal swingset-core-eval supports mutiple permit/script pairs.

And likewise some code in @agoric/synthetic-chain, IIRC.

@dckc
Copy link
Member

dckc commented Aug 8, 2024

We don't have CLI support to create such a tx,

I'm not sure why you say that. agd supports it just fine, IME.

@LuqiPan
Copy link
Contributor

LuqiPan commented Aug 8, 2024

@dckc suggests using the promise space available in the bootstrap environment ...

Any additional testing considerations with the promise space approach?

@mhofman
Copy link
Member Author

mhofman commented Aug 8, 2024

we don't need multiple cosmos messages to run multiple scripts.

We do need multiple messages to support sequential execution of the core-eval scripts

I'm not sure why you say that. agd supports it just fine, IME.

agd currently only supports generating a governance proposal with a single CoreEvalProposal message.

Is there a reason that approach wouldn't just work?

It requires changing the runtime logic of the core evals. I consider that much more complicated and risky,

@dckc
Copy link
Member

dckc commented Aug 8, 2024

We do need multiple messages to support sequential execution of the core-eval scripts

But we don't need to do them sequentially.

It requires changing the runtime logic of the core evals. I consider that much more complicated and risky,

Really? I don't. And CH's comment suggests he doesn't either.

@dckc
Copy link
Member

dckc commented Aug 8, 2024

Any additional testing considerations with the promise space approach?

I don't think so; not beyond migrating the existing chain-software-upgrade-based tests to a core-eval-based test.

In another PR, we're discussing how existing tests exercise synchronization constraints: #9748 (comment)

@Chris-Hibbert
Copy link
Contributor

Any additional testing considerations with the promise space approach?

We can convert the existing software upgrade proposal in a3p-integration to a coreEval, and the tests will continue to work. If we add the promise dependency, they should run in order, and tests would be expected to pass.

@mhofman
Copy link
Member Author

mhofman commented Aug 9, 2024

Well apparently the new v1 gov proposal messages do not support CoreEvalProposal type, so looks like we won't be able to use the cosmos facilitated approach.

Filed #9869

@Chris-Hibbert
Copy link
Contributor

#9911 linked two proposals using promises in the promise space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants