gno icon indicating copy to clipboard operation
gno copied to clipboard

feat: utilize params keeper for valset updates

Open zivkovicmilos opened this issue 9 months ago β€’ 5 comments

Description

This PR introduces a new implementation for onchain valset management (v3), that utilizes the params keeper for propagating valset changes out to the SDK side.

We've went through a few iterations of onchain valset management:

  • valset changes are emitted as events, and caught in the EndBlocker. All information was propagated through events.
  • valset changes notified through events, but remained on-chain (in the Realm). Notifications were caught in EndBlocker, and we ran the GnoVM to fetch the state (changes) on-chain.
  • (this implementation) valset changes are saved in the VM param keeper, as part of the realm params. The EndBlocker logic reads the param, and if it's set for the given height, applies the changes.

The commonality for all of these different techniques was that we've required a GovDAO proposal to change the valset. This was unchanged in any iteration.

How it works

A GovDAO member makes a proposal to the GovDAO.

In the proposal, they prepare the valset changes they want to propose, with the information:

  • Validator address (bech32)
  • Validator public key (bech32)
  • Voting power (number)

The semantics are:

  • If the validator exists, and the voting power is different, the proposal is to change the validator's voting power.
  • If the validator doesn't exist, the proposal is to add the validator to the set.
  • If the validator exists, and the voting power is 0, the proposal is to remove the validator from the set.

Inside the executor body, the realm (gno.land/r/sys/validators/v3) updates its local representation (valset), but also sets the realm-local parameter: new_updates_available, and valset_new.

This realm-local params will be stored under the following keys in the global params keeper:

  • vm:gno.land/r/sys/validators/v3:new_updates_available (bool)
  • vm:gno.land/r/sys/validators/v3:valset_new ([]string)

As for the value of valset_new, we store a custom serialized representation of the change, since it needs to be read from EndBlocker (go code): address:pubkey:power

We store the changes as []string, so an example key-value for a valset change would be:

  • Key: vm:gno.land/r/sys/validators/v3:valset_new
  • Value: [g1vywlz0dvjl88yffephcune8eueuxqge3cwxnsm:gpub1pgfj7ard9eg82cjtv4u4xetrwqer2dntxyfzxz3pqgavqhgxg6j648ah2m4xgx8hcyv0fvlc6ajkaahafnq0hqufmuzxw26ev70:1] (the square brackets denote this is a []string in this explainer)

Once the block execution finishes, the EndBlocker logic runs, that checks if any valset changes were saved for the current height.

  • If they exist, they are extracted and applied
  • If they don't exist, there are no changes to apply

The flow in EndBlocker is as follows:

  1. Check if vm:gno.land/r/sys/validators/v3:new_updates_available is set to true. If not, no changes to apply.
  2. Grab the current valset by reading the serialization at vm:gno.land/r/sys/validators/v3:valset_prev.
  3. Grab the newly proposed valset by reading the serialization at vm:gno.land/r/sys/validators/v3:valset_new.
  4. Compare the current valset and proposed valset to extract the changes.
  5. Set the param vm:gno.land/r/sys/validators/v3:new_updates_available to false.
  6. Set the param vm:gno.land/r/sys/validators/v3:valset_prev to be the value of vm:gno.land/r/sys/validators/v3:valset_new (they match).
  7. Propagate the changes out to the SDK.

Misc changes in this PR

  • I've added support for fetching params from the SDKParams implementation.
  • Added a sys param called ValsetRealmPath to the VM params. This field denotes the canonical realm path of the valset realm to be monitored (for params changes).

zivkovicmilos avatar Mar 24 '25 10:03 zivkovicmilos

πŸ›  PR Checks Summary

πŸ”΄ Must not contain the "don't merge" label

Manual Checks (for Reviewers):
  • [ ] IGNORE the bot requirements for this PR (force green CI check)
Read More

πŸ€– This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

βœ… Automated Checks (for Contributors):

πŸ”΄ Must not contain the "don't merge" label

β˜‘οΈ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
β˜‘οΈ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
πŸ“š Resources:
Debug
Automated Checks
Must not contain the "don't merge" label

If

🟒 Condition met
└── 🟒 A label matches this pattern: don't merge (label: don't merge)

Then

πŸ”΄ Requirement not satisfied
└── πŸ”΄ On no pull request

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟒 Condition met
└── 🟒 On every pull request

Can be checked by

  • Any user with comment edit permission

Gno2D2 avatar Mar 24 '25 10:03 Gno2D2

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests.

:loudspeaker: Thoughts on this report? Let us know!

codecov[bot] avatar Mar 24 '25 11:03 codecov[bot]

After a general discussion with Jae regarding x/params (related to this PR, Guilhem’s, the one already merged for restricted accounts, and upcoming ones), we’ve aligned on a direction to make x/params usage:

  1. Write-only by contracts, read-write by the chain β€” this contrasts with the current assumption (possibly made in this PR) that the chain mostly reads from it.
  2. Reflect long-term values, not transient state β€” params should hold meaningful, stable data.

For this specific valset PR, Jae proposed a smart and simple pattern using three variables:

  1. new_updates_available bool β€” always set to true by the contract when there’s a change, and always reset to false by the chain after processing the update.
  2. valset_new []valset_entry β€” the target validator set, written by the contract, read (but never modified) by the chain. This is what we want to reach.
  3. valset_prev []valset_entry β€” written by the chain after applying the update. This reflects the current validator set at the start of the block (or most recently applied).

Typically, valset_new and valset_prev are the same, except during the block where updates are being processed.

This approach keeps the contract simple: it just maintains a local view of the desired validator set and writes it into params. The chain handles:

  • Checking the new_updates_available flag during the end blocker (cheap),
  • Comparing valset_new and valset_prev to determine updates,
  • Writing back to valset_prev.

This works:

  • In real-time,
  • With multiple changes in a single block,
  • Across restarts,
  • During snapshot replays,
  • For safe query behavior.

Simple, robust, and clear separation of roles between contracts and chain.

moul avatar Apr 09 '25 19:04 moul

@moul

I've applied the suggestions from the comment πŸ™ Let me know if the approach is good now, and I'll resolve the tests / examples / leftover threads

zivkovicmilos avatar Apr 24 '25 14:04 zivkovicmilos

In the context of the IBC protocol, this std.SetParam can be useful since it allows to have determistic key paths. On the contrary, objects stored in the global vars of a realm, the key paths are difficult to determine, if not impossible currently.

However, what's missing for IBC is the ability to delete these keys from params. Is there any chance of adding std.DelParam ?

tbruyelle avatar Jun 20 '25 12:06 tbruyelle

Is this PR frozen for some reason ?

tbruyelle avatar Jul 09 '25 10:07 tbruyelle

Pr is not frozen, we should do it.

std.DelParam() can be added πŸ‘

Making realms’ var more easily queryable is also a goal.

moul avatar Jul 09 '25 18:07 moul