Skip to content

Remove unused default value in StorageMaps to allow OptionQuery#210

Merged
haerdib merged 2 commits intomasterfrom
bh-allow-option-query
Feb 8, 2022
Merged

Remove unused default value in StorageMaps to allow OptionQuery#210
haerdib merged 2 commits intomasterfrom
bh-allow-option-query

Conversation

@haerdib
Copy link
Contributor

@haerdib haerdib commented Feb 8, 2022

Removes unused default value from the StorageMaps. It was not used and blocked OptionQueries by throwing MapValueTypeErrors when querying an Option Value.

Tested with recent node and worker (integritee-network/worker#645)

@haerdib haerdib requested a review from clangenb February 8, 2022 13:49
@haerdib haerdib self-assigned this Feb 8, 2022
Copy link
Collaborator

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Looks so far good to me.

So, is there a difference now in how you call an OptionQuery map or an ValueQuery? If yes, it would be valuable to have that documented somewhere

@haerdib
Copy link
Contributor Author

haerdib commented Feb 8, 2022

No, there is no difference, actually. With the removal of the default value, it retrieves the optional storage value perfectly without any changes on the client side.

Copy link
Collaborator

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Cool, then I approve as is.

@haerdib haerdib merged commit 7b6b822 into master Feb 8, 2022
@haerdib haerdib deleted the bh-allow-option-query branch February 8, 2022 14:31
@haerdib haerdib added F8-newfeature Introduces a new feature E1-breaksnothing labels Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

E1-breaksnothing F8-newfeature Introduces a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants