-
Notifications
You must be signed in to change notification settings - Fork 350
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 storage deposit support #4304
Merged
jacogr
merged 12 commits into
polkadot-js:master
from
statictype:ae-contracts-storageDeposit
Dec 14, 2021
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
10eea63
add storage deposit support
statictype 8bdf95f
Revert "add storage deposit support"
statictype da9afac
Merge branch 'master' into ae-contracts-storageDeposit
statictype 96dbb54
generate new metadata
statictype 4c70ba0
make it build
statictype 2a5f873
add storage deposit in RPCs
statictype ebfc663
rename endowment to value
statictype 9eaf0c0
support older nodes
statictype 1f9e287
check the right things
statictype e3ff8a3
hide storage_deposit until better supported
statictype f5d3d03
add the right defaults
statictype 32e74ec
hardcode storageDepositLimit to null
statictype File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backwards compat is always a HUGE issue. So, in this case (luckily) the number of paramaters changed, so it would be a case of -
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change necessary in case api-contract version is different than api version?
it would mean overloading the generated interfaces somehow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IT is needed since it depends on the chain it is connected to - so it needs to be detected via on-chain metadata. You can have the most recent API with an ancient node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see... tried a few things yesterday but was unable to figure out how to do that. is there a PR where you did something similar that you can point me to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, it is a bit of a nightmare, since these things are short-lived - so I generally allow 3+ months leeway for people to upgrade before ripping it out, so the examples that were here (contracts specific) have been removed.
Will dig around the older versions and find it. (There is a similar approach in the apps UI as well, which tries to also give around 3 months.... but generally they end up living a "bit" longer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is an example for the apps UI that does the same thing -
https://github.com/polkadot-js/apps/blob/master/packages/page-democracy/src/Overview/Fasttrack.tsx#L59-L63
There are millions of them in there... sadly...
Mostly in the API it is always only contracts since it is the one place where we add convenience wrappers around lower-level interfaces... will find an old release having some.
EDIT: Here is the 4.9.1 api release with the blueprint deploy still supporting 2 versions (it was removed in 4.10.1, aka grace-period ended... the apps UI really needs to learn from this and start cleaning up after itself...)
api/packages/api-contract/src/base/Blueprint.ts
Lines 55 to 71 in 23d5024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it now! you have to
ts-ignore
the old styleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is slightly messy code-wise :(
… but better than dealing with the “it doesn’t work-anymore” flood of support…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i got everything but still can't call. I get this rpc core error
polkadot-js/apps#6657 (comment)
There is also this contract query, but not sure if adding an extra param there would solve it and what that param should be
https://github.com/polkadot-js/apps/blob/db4f91e47531fa4c6d00c4015dcbadae2deb5683/package/page-contracts/src/shared/Messages.tsx#L67