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

Add marshaller to data trie tracker #4640

Merged

Conversation

BeniaminDrasovean
Copy link
Contributor

Reasoning behind the pull request

  • The trackable data trie needs a marshalizer in order to serialize the data that will be inserted in the trie

Proposed changes

  • Add marshalizer to trackable data trie

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@codecov-commenter
Copy link

Codecov Report

Base: 70.72% // Head: 70.72% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (9bc791f) compared to base (c279ee4).
Patch coverage: 80.00% of modified lines in pull request are covered.

Additional details and impacted files
@@                   Coverage Diff                    @@
##           feat/balance-data-tries    #4640   +/-   ##
========================================================
  Coverage                    70.72%   70.72%           
========================================================
  Files                          625      625           
  Lines                        83128    83131    +3     
========================================================
+ Hits                         58794    58797    +3     
  Misses                       19926    19926           
  Partials                      4408     4408           
Impacted Files Coverage Δ
factory/processing/processComponents.go 0.00% <0.00%> (ø)
update/genesis/base.go 24.13% <0.00%> (ø)
update/genesis/import.go 13.33% <0.00%> (ø)
process/block/baseProcess.go 73.34% <100.00%> (ø)
state/accountsDB.go 78.96% <100.00%> (ø)
state/factory/accountCreator.go 100.00% <100.00%> (ø)
state/factory/peerAccountCreator.go 100.00% <100.00%> (ø)
state/peerAccount.go 84.41% <100.00%> (ø)
state/trackableDataTrie.go 87.50% <100.00%> (+0.61%) ⬆️
state/userAccount.go 90.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

require.Nil(t, err)
err = rwd.userAccountsDB.SaveAccount(peerAccount)
require.Nil(t, err)
isDelegationSCAddress = rwd.isSystemDelegationSC(peerAccount.AddressBytes())
require.False(t, isDelegationSCAddress)

// existing user account
userAccount, err := state.NewUserAccount([]byte("userAddress"), &hashingMocks.HasherMock{})
userAccount, err := state.NewUserAccount([]byte("userAddress"), &hashingMocks.HasherMock{}, &marshal.GogoProtoMarshalizer{})
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we use here the mock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

dirtyData map[string][]byte
tr common.Trie
hasher hashing.Hasher
marshalizer marshal.Marshalizer
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to marshaller ?

@bogdan-rosianu bogdan-rosianu self-requested a review October 25, 2022 12:15
bogdan-rosianu
bogdan-rosianu previously approved these changes Oct 25, 2022
@BeniaminDrasovean BeniaminDrasovean merged commit 535dd32 into feat/balance-data-tries Oct 26, 2022
@BeniaminDrasovean BeniaminDrasovean deleted the add-marshaller-to-data-trie-tracker branch October 26, 2022 07:12
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.

4 participants