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.

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

Update by reference for two state objects affect each other #3925

Open
mkalinin opened this issue Sep 17, 2024 · 3 comments
Open

Update by reference for two state objects affect each other #3925

mkalinin opened this issue Sep 17, 2024 · 3 comments
Labels
general:bug Something isn't working

Comments

@mkalinin
Copy link
Collaborator

mkalinin commented Sep 17, 2024

Consider the following test:

@with_electra_and_later
@spec_state_test
def test_state_changes_overwritten(spec, state):
    validator_0 = state.validators[0]
    validator_1 = state.validators[1]
    
    validator_0.exit_epoch = spec.Epoch(0)
    assert state.validators[0].exit_epoch == spec.Epoch(0)

    validator_1.exit_epoch = spec.Epoch(1)

    # Check that both changes have been applied to the state
    assert state.validators[1].exit_epoch == spec.Epoch(1)
    assert state.validators[0].exit_epoch == spec.Epoch(0)

The second assert fails, as validator_1.exit_epoch = spec.Epoch(1) reverts the change made to validator_0 in the line above. If we change the order to 1) update validator_1 2) update validator_0 then the first assert would fail.

@mkalinin
Copy link
Collaborator Author

The cause of the bug is in the following remerkleable behaviour:

  1. state.validators[0] expression creates the following chain of view backings: state.validators_a.validator_0 and keeps it in the validator_0 view, similarly validator_1 denotes state.validators_b.validator_1
  2. validator_0.exit_epoch = spec.Epoch(0) assignment updates validator_0 chain of backings to state_a.validators_a.validator_0_updated, note that the state view is now backed by the state_a backing and the first assert passes. Also, note that validator_1 denotes state_a.validators_b.validator_1: the validators view diverged from the state view
  3. validator_1.exit_epoch = spec.Epoch(1) assignment is applied to validator_1, then it updates validators backing and is finally propagated to the state backing, resulting in the following chain of backings: state_b.validators_b.validator_1 and the state now backed by state_b making the last assert fail

Potential solution would be to cache a child view inside of a parent view instance if a child view is a ComplexView that can be further updated by reference; and on every subsequence calls obtaining the same child view return the cached value.

With this approach the above case would have: validator_0 <- state.validators.validator_0, and validator_1 <- state.validators.validator_1. So, both validator views would refer to the same instance of state.validators. And an update to validator_0 would be propagated to validator_1 backings

@mkalinin
Copy link
Collaborator Author

Another case with similar cause:

@with_electra_and_later
@spec_state_test
def test_state_changes_overwritten_2(spec, state):
    validator_0 = state.validators[0]
    state.validators[0].exit_epoch = spec.Epoch(0)

    assert validator_0.exit_epoch == spec.Epoch(0)

The assert above fails because the update is not applied to the validator_0 view

@ralexstokes
Copy link
Member

I wonder if @protolambda has anything to add here

@hwwhww hwwhww added the general:bug Something isn't working label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants