Skip to content

NEO: clear LastGasPerVote when voting for NULL, fix #2894#3173

Merged
shargon merged 3 commits intoneo-project:masterfrom
roman-khimov:clear-lastgas-on-unvote
Mar 7, 2024
Merged

NEO: clear LastGasPerVote when voting for NULL, fix #2894#3173
shargon merged 3 commits intoneo-project:masterfrom
roman-khimov:clear-lastgas-on-unvote

Conversation

@roman-khimov
Copy link
Contributor

@roman-khimov roman-khimov commented Mar 4, 2024

It's a rather trivial fix, but it needs some substantial testing wrt account balances (just comparing the state won't be enough). @superboyiii.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • Unit tests

@roman-khimov
NEO: clear LastGasPerVote when voting for NULL, fix neo-project#2894
5b912fe 
This value won't be used in any way, so save some bytes of storage.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
vncoelho
vncoelho reviewed Mar 4, 2024
View reviewed changes
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember this discussion some time ago. It seems to be ok.

@superboyiii
Copy link
Member

superboyiii commented Mar 5, 2024

I will

shargon
shargon approved these changes Mar 5, 2024
View reviewed changes
Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me, just wait for @superboyiii

@superboyiii
Copy link
Member

superboyiii commented Mar 7, 2024

Although it will surely make some storage changes, but checked NEO and GAS balance on all mainnet existed account, no problem. So it should be safe.

@shargon
Copy link
Member

shargon commented Mar 7, 2024

Although it will surely make some storage changes, but checked NEO and GAS balance on all mainnet existed account, no problem. So it should be safe.

Did you checked?

@superboyiii
Copy link
Member

superboyiii commented Mar 7, 2024 •
edited
Loading

Although it will surely make some storage changes, but checked NEO and GAS balance on all mainnet existed account, no problem. So it should be safe.

Did you checked?

Yes, I checked. Many storage changes there. But the same event log(checked some of these) and the same NEO and GAS balance(all existed addresses).
78fbcefe59b9de71534b29303193711

For example, in a tx: 0x38091b803f794e50dcc10a9091becaf4f65d35d3ef9e71cfa90c7936af50757e on block 39632 of mainnet,
Original Storage Changes:

{
    "block": 39632,
    "size": 7,
    "storage": [
        {
            "state": "Changed",
            "key": "\u002Bv///xQXVU6kNdnigqZEh4tAVHW\u002BNwA2AQ==",
            "value": "QQEhBCg32Dw="
        },
        {
            "state": "Changed",
            "key": "\u002Bv///ws=",
            "value": "68Z3AaV6Eg=="
        },
        {
            "state": "Changed",
            "key": "\u002Bv///xSPX4PVGdD5t8CX0nYRsTB3r0935Q==",
            "value": "QQEhBbkBXAAW"
        },
        {
            "state": "Changed",
            "key": "\u002B////xQXVU6kNdnigqZEh4tAVHW\u002BNwA2AQ==",
            "value": "QQQhAlEEIQPQmgAAIQfI7lv90/cA"
        },
        {
            "state": "Changed",
            "key": "\u002B////wE=",
            "value": "Riw0AQ=="
        },
        {
            "state": "Changed",
            "key": "\u002B////yED/QTemD9OBMlimrPPyD9BvnQxuWv4UqkYc8OMqPc37iw=",
            "value": "QQIgASEDKV8C"
        },
        {
            "state": "Changed",
            "key": "\u002Bv///xRjslsjaXYnEvQP2GT6JumaYZgBIw==",
            "value": "QQEhBeYy2gAW"
        }
    ]
}

In this PR:

{
    "block": 39632,
    "size": 7,
    "storage": [
        {
            "state": "Changed",
            "key": "\u002Bv///xQXVU6kNdnigqZEh4tAVHW\u002BNwA2AQ==",
            "value": "QQEhBCg32Dw="
        },
        {
            "state": "Changed",
            "key": "\u002Bv///ws=",
            "value": "68Z3AaV6Eg=="
        },
        {
            "state": "Changed",
            "key": "\u002Bv///xSPX4PVGdD5t8CX0nYRsTB3r0935Q==",
            "value": "QQEhBbkBXAAW"
        },
        {
            "state": "Changed",
            "key": "\u002B////xQXVU6kNdnigqZEh4tAVHW\u002BNwA2AQ==",
            "value": "QQQhAlEEIQPQmgAAIQA="
        },
        {
            "state": "Changed",
            "key": "\u002B////wE=",
            "value": "Riw0AQ=="
        },
        {
            "state": "Changed",
            "key": "\u002B////yED/QTemD9OBMlimrPPyD9BvnQxuWv4UqkYc8OMqPc37iw=",
            "value": "QQIgASEDKV8C"
        },
        {
            "state": "Changed",
            "key": "\u002Bv///xRjslsjaXYnEvQP2GT6JumaYZgBIw==",
            "value": "QQEhBeYy2gAW"
        }
    ]
}

So the differ is between the storage of key \u002B////xQXVU6kNdnigqZEh4tAVHW\u002BNwA2AQ==
Original: QQQhAlEEIQPQmgAAIQfI7lv90/cA
VS
this PR: QQQhAlEEIQPQmgAAIQA=
1709805858089
But not sure what's stored in this key, could you help me check what's in this storage? @shargon

@roman-khimov
Copy link
Contributor Author

roman-khimov commented Mar 7, 2024

Exactly as expected, it's LastGasPerVote that's not going to be used (because account doesn't vote). So we store 0 (short) instead of something (more bytes).

@shargon shargon merged commit b05501a into neo-project:master Mar 7, 2024
@AnnaShaleva AnnaShaleva mentioned this pull request Mar 8, 2024
Closed
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 13, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
86bea33 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
@AliceInHunterland AliceInHunterland mentioned this pull request Mar 13, 2024
Merged
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 13, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
5456d9c 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 14, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
79da77d 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 14, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
399ec60 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 14, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
858f215 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 14, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
2b8254a 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 14, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
ac184c3 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 15, 2024
@AliceInHunterland
native: clear LastGasPerVote when voting for NULL
0e6fbad 
Port neo-project/neo#3173.

Close #3345

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vncoelho vncoelho vncoelho left review comments

@shargon shargon shargon approved these changes

@Jim8y Jim8y Jim8y approved these changes

@AnnaShaleva AnnaShaleva AnnaShaleva approved these changes

+1 more reviewer

@superboyiii superboyiii superboyiii approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@roman-khimov @superboyiii @shargon @Jim8y @vncoelho @AnnaShaleva