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

Delete nil esdt data #2980

Merged
merged 2 commits into from
Apr 9, 2021
Merged

Conversation

sasurobert
Copy link
Contributor

Delete nil esdt data

@iulianpascalau iulianpascalau self-requested a review April 9, 2021 07:20
@bogdan-rosianu bogdan-rosianu self-requested a review April 9, 2021 07:21
func saveESDTData(
userAcnt state.UserAccountHandler,
esdtData *esdt.ESDigitalToken,
key []byte,
marshalizer marshal.Marshalizer,
) error {
isValueZero := esdtData.Value.Cmp(zero) == 0
if isValueZero && arePropertiesEmpty(esdtData.Properties) {
return userAcnt.DataTrieTracker().SaveKeyValue(key, nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

will this remove the key also? or the key remains but with null value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this removes the key.

@@ -265,12 +265,26 @@ func checkFrozeAndPause(
return nil
}

func arePropertiesEmpty(properties []byte) bool {
for _, property := range properties {
Copy link
Contributor

Choose a reason for hiding this comment

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

could use an early if len(properties) > 0 { return false }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. as if you do len(make([]byte, 10)) it returns 10

@sasurobert sasurobert merged commit 1e9f376 into feat/eip-esdt-local-mint Apr 9, 2021
@sasurobert sasurobert deleted the delete-nil-esdt-data branch April 9, 2021 07:29
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.

None yet

3 participants