-
Notifications
You must be signed in to change notification settings - Fork 207
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
Cosmos init action may contain non-consensus data #9946
Labels
Comments
mhofman
added
enhancement
New feature or request
cosmic-swingset
package: cosmic-swingset
agoric-cosmos
labels
Aug 22, 2024
gibson042
added a commit
that referenced
this issue
Aug 28, 2024
gibson042
added a commit
that referenced
this issue
Aug 29, 2024
siarhei-agoric
pushed a commit
that referenced
this issue
Aug 30, 2024
siarhei-agoric
pushed a commit
that referenced
this issue
Aug 30, 2024
gibson042
added a commit
that referenced
this issue
Aug 30, 2024
siarhei-agoric
pushed a commit
that referenced
this issue
Aug 30, 2024
mergify bot
added a commit
that referenced
this issue
Aug 30, 2024
…for cosmic-swingset (#9987) Fixes #9946 ## Description * Share runtime cosmos-sdk [viper] configuration with cosmic-swingset in the AG_COSMOS_INIT message * Exclude non-consensus configuration from bootstrap vat arguments ### Security Considerations I believe that cosmic-swingset should be considered as a peer of the Go parts of agd, and thus entitled to full runtime configuration. If we have reason to consider it otherwise, then filtering will be warranted. ### Scaling Considerations This increases the size of AG_COSMOS_INIT messages by up to several KB (but those messages are one-per-process-start). ### Documentation Considerations We'll want cross-referencing of any config values that are accessed by cosmic-swingset. ### Testing Considerations TBD ### Upgrade Considerations This changes the bootstrap vat arguments (which are subject to consensus) for any new chain instance to exclude the "port" settings. However, it should not affect any existing chain, and the bootstrap vat does not use that configuration.
siarhei-agoric
pushed a commit
that referenced
this issue
Aug 30, 2024
siarhei-agoric
pushed a commit
that referenced
this issue
Aug 31, 2024
mergify bot
added a commit
that referenced
this issue
Aug 31, 2024
) closes: #9574 refs: #9946 ## Description This change set builds on top of PR #9987 to plumb maxVatsOnline into JS side of SwingSet. ### Security Considerations A new locally-customizable configuration item is being passed to SwingSet from golang via initAction message. ### Scaling Considerations New config variable is read from a local config file, parsed, and included in the initAction message. However, the variable is not part of consensus and passed along to the SwingSet only once. ### Documentation Considerations new variable `maxVatsOnline` is added to `[swingset]` section of `config/app.toml` ### Testing Considerations Tested manually by watching total number of vats running as measured by looking at ps aux | grep -c 'xsnap-worker v' while running make scenario2-run-chain for different values of maxVatsOnline in the config. ### Upgrade Considerations None, since the default value is the same as previously hardcoded one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
What is the Problem Being Solved?
The cosmos init action is currently included as-is as the
bootMsg
passed in the bootstrap vat's parameters. As a message queued in swingset, it affects consensus.However some fields in the init action (e.g. the ports) may not have determinism guarantees, and we're considering adding a
swingsetConfig
field to the init action to support #9386 and #9574Description of the Design
A clear separation in the init message between consensus data that should end up in
bootMsg
and non-consensus data that should not.Security Considerations
Maintain consensus
Scaling Considerations
None
Test Plan
Bootstrapping a multinode chain with different local swingset config or other locally decided fields
Upgrade Considerations
None
The text was updated successfully, but these errors were encountered: