Add storage deposit support

[statictype]

An attempt to add support for storage deposits that were introduced in the contracts pallet with this PR

Not sure what to do for backwards compatibility.

This is the first step, a PR in polkadot-js/apps will follow where i implement the input for storage deposit.
I will rename endowment in a following commit here

[jacogr]

Backwards compat is always a HUGE issue. So, in this case (luckily) the number of paramaters changed, so it would be a case of -

this.api.tx.contracts.instantiate.meta.args.length === 6
  ? this.api.tx.contracts.instantiate(value, gasLimit, storageDepositLimit, codeHash, params, salt)
  : this.api.tx.contracts.instantiate(value, gasLimit, codeHash, params, salt)

[statictype]

is this change necessary in case api-contract version is different than api version?
it would mean overloading the generated interfaces somehow.

[jacogr]

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.

[statictype]

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?

[jacogr]

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)

[jacogr]

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

	#deploy = (constructorOrId: AbiConstructor | string | number, { gasLimit = 0, salt, value = 0 }: BlueprintOptions, params: CodecArg[]): SubmittableExtrinsic<ApiType, BlueprintSubmittableResult<ApiType>> => {
	const encodedSalt = encodeSalt(salt);
	const withSalt = this.api.tx.contracts.instantiate.meta.args.length === 5;
	const encoded = this.abi.findConstructor(constructorOrId).toU8a(params, withSalt ? EMPTY_SALT : encodedSalt);
	const tx = withSalt
	? this.api.tx.contracts.instantiate(value, gasLimit, this.codeHash, encoded, encodedSalt)
	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
	// @ts-ignore old style with salt included
	: this.api.tx.contracts.instantiate(value, gasLimit, this.codeHash, encoded);
	return tx.withResultTransform((result: ISubmittableResult) =>
	new BlueprintSubmittableResult(result, applyOnEvent(result, ['Instantiated'], ([record]: EventRecord[]) =>
	new Contract<ApiType>(this.api, this.abi, record.event.data[1] as AccountId, this._decorateMethod)
	))
	);
	}
	}

[statictype]

got it now! you have to ts-ignore the old style

[jacogr]

Yes, it is slightly messy code-wise :(

… but better than dealing with the “it doesn’t work-anymore” flood of support…

[statictype]

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

[statictype]

it seems that when i do yarn build locally some of my changes are reverted.
i couldn't find the entry point where all the interfaces are generated from. do I need to modify this too? https://github.com/polkadot-js/api/blob/master/packages/types-support/src/metadata/v14/substrate-types.json#L14629

[statictype]

actually, i think something like #4286 will do the trick. but don't know what the bump process is exactly

[jacogr]

It is supposed to be documented here, but don't think it had much attention lately -

https://github.com/polkadot-js/api/blob/master/packages/types/src/metadata/README.md

Small differences from the above - for substrate it is in v14/substrate.ts (instead of static.ts, kusama/polkadot dev now have their own as well)

Once you run yarn builld:interfaces it will re-generate all the TS stuff, the yarn test:one packages/types/src/metadata/v14 will do the rest of the checks.

TL;DR Get hex from chain, copy in, run build.

[statictype]

is there a way i can run local apps with local api to verify my changes?

[jacogr]

To check local changes -

• assume you have a root folder, let's call it ROOT
• checkout the repos in ROOT/api and ROOT/apps
• cd ROOT/api
• yarn polkadot-dev-copy-to apps

That will build and copy the changes to the apps UI, replacing what is there

• cd ../apps
• yarn start

It works for all projects here, so you can (from inside e.g. common) copy to api (or any relative folder, it assumed it is one level down, but also use it a bit for stuff like yarn polkadot-dev-copy-to ../test/some-bug-replication)

[statictype]

instantiating and calling contracts work now on old and new node
i had to hide storage_deposit and storage_deposit_limit completely from rpc type definitions because of back compatibility.
when the type comes from metadata the value is hardcoded to null

it works, but the subtracted fee is not visible in the UI at all.
to properly support these new features more things need to be done, but this might work for now. what do you think? @jacogr

still to do

• getting the estimation for storage deposit (and gas limit) when instantiating - not a small issue
• fix InputBalance problem with accepting default values in some cases (works only in the upload modal) and actually showing the input

[athei]

getting the estimation for storage deposit (and gas limit) when instantiating - not a small issue

Can we do this in a follow up? This might be out of scope (we hadn't had this before). There is a separate issue for that.

[statictype]

a follow-up was exactly what i was thinking

[jacogr]

Agreed with splitting - smaller PRs is always better instead of trying to re-invent the whole world. Plus merging moves things forward.

[jacogr]

Seems sane :) Thanks an absolute million, this is great.

[polkadot-js-bot]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.