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

Voting to NULL ("unvoting") in NEO doesn't clear LastGasPerVote #2894

Closed
roman-khimov opened this issue Sep 5, 2023 · 1 comment
Closed
Milestone

Comments

@roman-khimov
Copy link
Contributor

Describe the bug
When some NEO owner removes his previously casted vote completely (that is he calls vote(acc, null)) the old LastGasPerVote value is kept in the account state. It won't be used in any way since VoteTo is null, but it still takes up some storage space.

Not critical, but would be nice to fix in 3.7.

Expected behavior
Clear LastGasPerVote when voting for null.

Current behavior

if (voteTo != null && voteTo != state_account.VoteTo)
{
StorageKey voterRewardKey = CreateStorageKey(Prefix_VoterRewardPerCommittee).Add(voteTo);
var latestGasPerVote = engine.Snapshot.TryGet(voterRewardKey) ?? BigInteger.Zero;
state_account.LastGasPerVote = latestGasPerVote;
}
ECPoint from = state_account.VoteTo;
state_account.VoteTo = voteTo;

(Optional) Additional context
Discovered accidentally with the master version of NeoGo, it had a state diff relative to released 3.6:

block 41660: value mismatch for key +////xTrvgat3qG/w8hQoD/I4MgUz6rygA==: QQQhAS8hA7yiAAAhAA== vs QQQhAS8hA7yiAAAhB+POSWfBCAE=

NeoGo tried to save an account state of

00000000  41 04 21 01 2f 21 03 bc  a2 00 00 21 00           |A.!./!.....!.|

while C# node has

00000000  41 04 21 01 2f 21 03 bc  a2 00 00 21 07 e3 ce 49  |A.!./!.....!...I|
00000010  67 c1 08 01                                       |g...|

(notice zero 21 00 vs value stored in 21 07 ... in the end)

But it's not because NeoGo has the "proper" behavior described above, it's just that it loses LastGasPerVote accidentally during deserialization of non-voting account (see nspcc-dev/neo-go#3122 fix). So both nodes can be improved.

Related to #2841.

This was referenced Sep 14, 2023
@roman-khimov
Copy link
Contributor Author

This one is really easy to do, but:

  • it requires a resync (but 3.7.0 is likely to require it anyway)
  • it requires some careful testing as there will be some changes to the state (though I don't expect a lot of them)

So either we include it into 3.7.0 or it couldn't be fixed after #1526 solution. Cast 👍 for inclusion into 3.7.0 below or 👎 for forgetting about it.

@roman-khimov roman-khimov added this to the v3.7.0 milestone Nov 21, 2023
roman-khimov added a commit to roman-khimov/neo that referenced this issue Mar 4, 2024
This value won't be used in any way, so save some bytes of storage.

Signed-off-by: Roman Khimov <[email protected]>
@shargon shargon closed this as completed in b05501a Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant