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.

Sign up for GitHub

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

Jump to bottom

EPIC: Simulator #14753

Closed
tac0turtle opened this issue Jan 24, 2023 · 9 comments
Closed

EPIC: Simulator #14753

tac0turtle opened this issue Jan 24, 2023 · 9 comments
Assignees
@alpe
Labels
C:Simulations T:Epic Epics

Comments

@tac0turtle
Copy link
Member

Summary

This EPIC is the start of a tracking issue for features we would like to have in a simulator. We should identify the pitfalls of the current design, if any, and aim to provide a better user experience

One feature that would be nice is extend the simulator from a dumb fuzzer to a more intelligent fuzzer where improper messages would be caught if they errored.

Work Breakdown

TBD

I'd like to invite people to comment what features they would like to see from the simulator
@julienrbrt julienrbrt mentioned this issue Jan 24, 2023
Merged
19 tasks
@alexanderbez
Copy link
Contributor

I know dYdX wants #13843

@robert-zaremba
Copy link
Collaborator

  1. Error reporting. Today, tracing an error in simulations is a super unfriendly.
  2. Better high level functions to register and handle operations. Today, there is lot of boilerplate to deliver an operation.

@gjermundgaraba
Copy link
Contributor

gjermundgaraba commented Feb 9, 2023 •
edited
Loading

I have implemented simulations from 0 this week, and I have some feedback:

  • I want much, much less boilerplate. I'd rather the simulation framework was much less flexible and just worked if I implemented a couple of interfaces. In particular, the whole thing falls apart after you implement the module simulation interfaces. It's so much stuff to wire together afterwards. It is also not great that I essentially have to duplicate a lot of logic into the operation functions so they don't crash and burn.
  • The whole setup of the runsim binary is confusing and seems unnecessary, especially since it doesn't output good error messages. I think I would much prefer to not have an extra binary, unless it somehow becomes the key to remove all the boilerplate needed.
  • I would like to have all things simulator related implementations be in one place. I don't want this concept spread all over the chain code. It is messy and hard to keep straight in your head.

If anyone is interested, I can link the PR for this change to illustrate how much appears to be needed, just get started.

Since this is more about features, I just leave my feedback on docs here at the bottom so they are somewhere at least:

  • Docs need to explain more deeply how simulations are supposed to work (or rather, how your implementations are supposed to behave)
  • Docs need to cover the whole process from A-Z, today it doesn't

@alexanderbez
Copy link
Contributor

This is great feedback @gjermundgaraba, thank you!

@alexanderbez alexanderbez self-assigned this Feb 10, 2023
@elias-orijtech
Copy link
Contributor

If anyone is interested, I can link the PR for this change to illustrate how much appears to be needed, just get started.

@gjermundgaraba I'm working on the simulator rewrite I'd like to see the PR you did to integrate simulation.

@gjermundgaraba
Copy link
Contributor

If anyone is interested, I can link the PR for this change to illustrate how much appears to be needed, just get started.

@gjermundgaraba I'm working on the simulator rewrite I'd like to see the PR you did to integrate simulation.

Happy to! Here it is :)
EmpowerPlastic/empowerchain#469

@elias-orijtech elias-orijtech mentioned this issue Nov 2, 2023
Closed
elias-orijtech added a commit to elias-orijtech/cosmos-sdk that referenced this issue Nov 10, 2023
@elias-orijtech
feat(sim): implement the beginning of a replacement simulator
dc967dd 
This change implements a replacement for the current simulator based
on testutil/network. Most of the changes are porting the module specific
message generators to no longer rely on SimulationState, and to generate
"real" messages, not simulator messages. The simulator driver is in
simapp, as part of the IntegationTestSuite.

The new approach aims to improve simulation in two important ways:

- Simulation should more closely mimic a real network. The current
simulator message delivery is implemented parallel to non-simulator
message delivery, leading to loss of fidelity and higher maintenance.
One symptom is cosmos#13843.
- Simulation should be layered on top of modules, not part of modules.
This means that modules should not import simulation packages, nor refer
to its generator package (x/module/simulation). This should eventually
fix cosmos#7622.

There are also downsides, however. Where the current simulator is too
high level, testutil/network is too low level: it runs a real network
of validators which is difficult to control. For example:

- AppHashes differ between runs, because modules may depend on non-
deterministic state such as block header timestamps.
- The validators runs in separate goroutines, which makes it hard to
query app state without introducing race conditions.
- Blocks are produced according tot time, and not under control by the
test driver. This makes it hard to trigger processing of messages in
particular blocks, which ruins determinism.

Some of the issues may be worked around, for example by forcing the
block headers to be deterministic; however, the real fix is to make
testutil/network itself deterministic, providing the goldilock level
of simulation: close enough to a real network, yet deterministic enough
to generate the same chain state for a given random seed.

A deterministic testutil/network is part of cosmos#18145.

Future work includes:

- Porting of the remaining module message generators.
- Generating (and verifying) deterministic AppHashes, allowing reliable
replay when a problematic message is detected. Depends on cosmos#18145.
- Save/reload of state for faster debugging cycles.
- Removal of the old simulator, most importantly the reference to it from
 module code.

 Updates cosmos#14753 (Simulator rewrite epic)
 Updates cosmos#7622 (reducing imports from modules to simulator)
 Updates cosmos#13843 (using real message delivery for simulation)
@fmorency
Copy link
Contributor

fmorency commented May 9, 2024 •
edited
Loading

I started adding simulation support in our chain and agree with @gjermundgaraba.

My main ask is a hands-on guide/tutorial that explains how to add simulator support to your CosmosSDK app.

Happy to help!

@tac0turtle tac0turtle assigned alpe and unassigned odeke-em and elias-orijtech May 9, 2024
@tac0turtle
Copy link
Member Author

thank you, @alpe is doing a cleanup right now with some new changes, after this we will create a tutorial on how to write simulations. :)

@github-project-automation github-project-automation bot moved this to 📋 Backlog in Cosmos-SDK Jun 3, 2024
@alpe alpe mentioned this issue Jun 4, 2024
Merged
12 tasks
@tac0turtle tac0turtle moved this from 📋 Backlog to 🤸‍♂️ In Progress in Cosmos-SDK Jul 19, 2024
@tac0turtle tac0turtle moved this from 🤸‍♂️ In Progress to 👀 Waiting / In review in Cosmos-SDK Sep 13, 2024
@tac0turtle
Copy link
Member Author

#21613

With this pr we have concluded the simulation cleanup. If you'd like new features or have feedback let us know. Thank you

@github-project-automation github-project-automation bot moved this from 👀 Waiting / In review to 🥳 Done in Cosmos-SDK Sep 16, 2024
@tac0turtle tac0turtle removed this from Cosmos-SDK Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alpe alpe

Labels
C:Simulations T:Epic Epics
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@alpe @robert-zaremba @fmorency @gjermundgaraba @odeke-em @alexanderbez @tac0turtle @elias-orijtech