namada icon indicating copy to clipboard operation
namada copied to clipboard

Removes custom established VPs

Open grarco opened this issue 1 year ago • 6 comments

Describe your changes

Closes #2554.

Modifies tx_init_account and tx_update account to not write the vp key. Updates of it are not supported for now and when initializing a new established account (also for validators) the protocol automatically assigns it the vp_user.

The host function update_validity_predicate has been removed.

The accounts' vps have been removed from the genesis files.

Indicate on which release or other PRs this topic is based on

v0.31.8

Checklist before merging to draft

  • [x] I have added a changelog
  • [x] Git history is in acceptable state

grarco avatar Feb 08 '24 11:02 grarco

@cwgoes do we really want to remove the possibility to update vps via governance? can't we just remove the possibility for usersd to update their acccou t vp and have just the default one? vps are whitelisted anyway and if we just remove the possibility from CLI and wasm transaction to update the validity predicate we can retain the possibility to create multiple of them via governance.

Fraccaman avatar Feb 12 '24 19:02 Fraccaman

@cwgoes do we really want to remove the possibility to update vps via governance? can't we just remove the possibility for usersd to update their acccou t vp and have just the default one? vps are whitelisted anyway and if we just remove the possibility from CLI and wasm transaction to update the validity predicate we can retain the possibility to create multiple of them via governance.

Yeah - I think we can retain the possibility to update VPs via governance, I just want to remove the user-submitted txs.

cwgoes avatar Feb 13 '24 07:02 cwgoes

Codecov Report

Attention: Patch coverage is 69.23077% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 54.00%. Comparing base (2535c9c) to head (3c54841).

Files Patch % Lines
crates/sdk/src/signing.rs 0.00% 4 Missing :warning:
crates/apps/src/lib/cli.rs 0.00% 2 Missing :warning:
crates/sdk/src/tx.rs 0.00% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2561      +/-   ##
==========================================
+ Coverage   53.95%   54.00%   +0.05%     
==========================================
  Files         308      308              
  Lines      100018    99759     -259     
==========================================
- Hits        53967    53879      -88     
+ Misses      46051    45880     -171     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Feb 13 '24 17:02 codecov-commenter

After #2770 we probably need to rethink this a bit since we might need to let user update their vps

grarco avatar Mar 05 '24 17:03 grarco

About my previous comment: given the way we chose to remove vps from the allowlist, I believe we need to revert the tx_update_account tx to allow the update of the vp in case it was removed from the allowlist. We can still prevent custom vps in genesis instead

grarco avatar Apr 23 '24 10:04 grarco

is this something we still want to include in the near future?

brentstone avatar Apr 27 '24 00:04 brentstone