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

Optimize vote reward data #2841

Merged
merged 7 commits into from
Mar 15, 2023
Merged

Conversation

ZhangTao1596
Copy link
Contributor

Close #2815

2nd option from #2815 (comment)

@ZhangTao1596
Copy link
Contributor Author

I will fix ut.

public override void FromStackItem(StackItem stackItem)
{
base.FromStackItem(stackItem);
Struct @struct = (Struct)stackItem;
BalanceHeight = (uint)@struct[1].GetInteger();
VoteTo = @struct[2].IsNull ? null : ECPoint.DecodePoint(@struct[2].GetSpan(), ECCurve.Secp256r1);
LastGasPerVote = @struct[3].GetInteger();
Copy link
Member

Choose a reason for hiding this comment

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

This will fork the chain

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes and no. We'll get a different NEO contract state, true. We may have some problem with getAccountState method, but that's easy to solve (strip the third struct item, return only the old two ones to stay compatible, we can add getAccountStateV2 that will return everything, but we might as well not do that, I don't see a lot of uses for this data). But anything else will just work: balances, transfer data, GAS calculations, etc. So it looks more like a regular DB resync to me.

@roman-khimov
Copy link
Contributor

The main question though, does it have any effect wrt to the DB resync speed and size? Have you measured this?

@ZhangTao1596
Copy link
Contributor Author

The main question though, does it have any effect wrt to the DB resync speed and size? Have you measured this?

Not yet. I just compare gas balances on Testnet. I will measure db size and resync speed these two days.

@ZhangTao1596
Copy link
Contributor Author

ZhangTao1596 commented Jan 12, 2023

I test data sync on my working laptop with RpcServer, ApplicationLogs, StateServicefullstate=true. Block height is from 0 to 1349498 on Testnet.

version sync time MPT data size block data size
this patch 5h38m 7.6G 886M
3.5.0 4h53m 8.8G 907M

I expect this patch will sync faster but actually not. I will retest it again on a server and test mainnet.

@ZhangTao1596 ZhangTao1596 changed the title Optimize vote reward Optimize vote reward data Feb 13, 2023
@ZhangTao1596 ZhangTao1596 reopened this Feb 13, 2023
@ZhangTao1596
Copy link
Contributor Author

Mainnet fullstate leveldb sync compare:

branch sync time(hour:min) MPT data size block data size
3.5.0 10:16 50G 2.4G
3.5.0 10:21 50G 2.4G
this patch 10:01 46G 2.3G
this patch 10:09 46G 2.3G

@roman-khimov
Copy link
Contributor

@shargon, @devhawk, @erikzhang, @vncoelho, @igormcoelho, I think after all of the testing done by @ZhangTao1596 here and in nspcc-dev/neo-go#2892 we can say that

  • it works
  • it makes the DB smaller
  • it improves sync time

So, I'd say it's good to go.

Copy link
Member

@superboyiii superboyiii left a comment

Choose a reason for hiding this comment

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

Tested OK. Checked NEO and GAS balance in recent block hiehgt for all addresses and it's compatible with 3.5.0 except additional value -LastGasPerVote return in getAccountState.
1676888059032

@superboyiii
Copy link
Member

@shargon I think we can move on.

@Jim8y
Copy link
Contributor

Jim8y commented Mar 4, 2023

@shargon may you please review this pr?

@shargon shargon merged commit df534f6 into neo-project:master Mar 15, 2023
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

Successfully merging this pull request may close these issues.

Efficiency of NeoToken VoterRewardPerCommittee records
5 participants