AleoNet / snarkVM

A Virtual Machine for Zero-Knowledge Executions
https://snarkvm.org
Apache License 2.0
1.08k stars 1.5k forks source link

Initializes the restrictions list for each network #2487

Closed vicsn closed 5 months ago

vicsn commented 5 months ago

Motivation

Mirror of: https://github.com/ProvableHQ/snarkVM/pull/15 Passed review and CI.

vicsn commented 5 months ago

I suspect if a restriction is added retroactively (on block ranges that have already been added to the ledger), it will corrupt the mappings for newly syncing nodes and result in a desync between those and currently synced nodes.

Caution indeed advised! But the above scenario is not possible because the node with updated list will not connect and sync blocks from nodes with other lists: https://github.com/AleoNet/snarkOS/pull/3306

Meshiest commented 5 months ago

But the above scenario is not possible because the node with updated list will not connect and sync blocks from nodes with other lists: AleoNet/snarkOS#3306

  1. validators sync to block 30k
  2. restriction added on block range 29k... for transfers for a certain address
  3. all nodes restart with the new restrictions
  4. incoming clients (or 1k behind validators) sync up to 29k and start hitting the newly restricted transfers
  5. validators that were at 30k have old mappings that do not reflect the restrictions, new clients/lagging validators have different mappings that reflect the restricted transactions
vicsn commented 5 months ago

Thank you for explaining @Meshiest . Indeed that sounds like a serious UX issue.

To further my own understanding and documentation: given that validators frequently lag multiple blocks behind one another even in a healthy network, with the current design "social consensus" would have to enforce restrictions at least 10 blocks into the future when any network restart happens which updates the restrictions list. Enforcing such a delay in code is not possible because a validator can't know independently if its behind tip or not, especially because other validators might have already been switched off for the restart.

apruden2008 commented 5 months ago

Approved under duress. I don't like allowing features which have not been tested. To me, If it doesn't have a test, it doesn't work.

Working on testing in canarynet now, per discussion with @damons