Add storage deposit support in apps #6657

statictype · 2021-12-10T14:12:21Z

Introduced in Substrate here

Done:

storageDepositLimit = null
rename endowment to value
update input info text
make value input zeroable where storageDepositLimit is present (updated nodes)

packages/page-contracts/src/Codes/Upload.tsx

statictype · 2021-12-12T22:34:40Z

instantiation works now on nodes with or without the latest changes, as far as I tested

i can't get call to work though, i keep getting this error when the modal opens

new node

old node

jacogr · 2021-12-13T17:44:58Z

The above one is problematic - the RPC results changed. The API have a __fallback specifier that may cater for this case. But basically the error suggests it cannot decode a number (I would guess... the bn.js errors def. have something exactly like that)

athei · 2021-12-16T11:32:15Z

packages/page-contracts/src/Codes/Upload.tsx

-              isZeroable={hasStorageDeposit}
+              isZeroable={contractAbi.constructors[constructorIndex].isPayable}


What does isZeroable do if true? Asking because zero could still be a valid value when the constructor/message is payable.

if it's true then 0 can be an acceptable value, otherwise the input validation fails. the input gets a red border and you can't move to the next step

now i tried with the older node and even if the constructor is not payable, the transaction fails with "contract not founded" if i pass 0.
is is safe to assume that if the node supports storage deposit than the value can be 0 for non payable constructors?

You have to differentiate between ABI (payable) and the node. ABI comes from ink! and is decoupled from the node version.

Old Node:
Always needs value/endowment in order to pay for rent. Disregard payable.

New Node:
Does not need any value except if the constructor is payable. Hence you should not even show the field and use 0 when the constructor isn't payable.

ink!:
Ink constructors are implicitly payable. The fuck up is that this is implicit. The field is missing from ABI. What you should do is interpete the absense of the payable field as payable = true. We intent to make it explicit and also allow to make them not payable. The following is forward compatible:
undefined -> true
false-> false
true -> true

ok, got it!
i am using this api.tx.contracts.instantiate.meta.args.length === 6 to check if the node has storage deposit or not. because the changes to value are related to storage deposit i wanted to double check it the same condition can be used

statictype · 2021-12-17T10:43:27Z

@jacogr this is ready for review, i think. is there something else we can do to get this available to the users faster?

jacogr · 2021-12-21T15:34:11Z

I'll get through the queue - I know I'm a bottlenck, it takes a while to get to stuff in the review queue. (In apps I currently have a large number of PRs > 1 month old which has not had a look, the queue should be at 0 and it runs longer than a page here, just because of not getting there. It has been quite neglected due to pressures elsewhere.)

statictype · 2021-12-21T19:18:00Z

whenever you have some time. build should pass now.

jacogr · 2021-12-22T05:15:15Z

On the list for today.

jacogr

lgtm code-wise, just some small comments.

The bumps here are weird, was it done manually?

jacogr · 2021-12-22T07:46:39Z

packages/page-contracts/src/Contracts/Deploy.tsx

@@ -50,6 +51,7 @@ function Deploy ({ codeHash, constructorIndex = 0, onClose, setConstructorIndex

  const [name, isNameValid, setName] = useNonEmptyString(code && code.json.name);
  const { contractAbi, errorText, isAbiError, isAbiSupplied, isAbiValid, onChangeAbi, onRemoveAbi } = useAbi([code && code.json.abi, code && code.contractAbi], codeHash, true);
+  const isConstructorPayable = contractAbi?.constructors[constructorIndex].isPayable !== false;


This would always be true. It does not get set on constructors at this point at all. So it will be undefined !== false in all cases.

the intention is to make the constructors explicitly payable / not payable in ink!
just trying to be forward compatible. but yes, it's an unnecessary check at this point.

I understand that, but it is actually not forward-compatible at all :) Until we have a plan for the actual ABI definitions, so at this point is is guessing... So, if we want to really do this - pass the flag here https://github.com/polkadot-js/api/blob/master/packages/api-contract/src/Abi/index.ts#L93 (like we do for messages)

For now it would need to always be isPayable: true. When the time comes that the constructors pass it, on the API level it would need to change that field into an Option<bool> for the decoders. The we need to change the always-true be a isPayable: o.isNone || o.unwrap().isTrue (to maintain backwards compat where it was not passed)

So not against the checks, however the tristate logic should not sit quite here. It is ok for now, but needs cleanup on the ABI level and then here.

jacogr · 2021-12-22T08:02:13Z

yarn.lock

+  languageName: node
+  linkType: hard
+
+"@polkadot/api-contract@npm:^7.0.3-0":


Somewhere things got mismatched between the versions here - the ./dev/all-update.sh script will ensure there is a single version and dedupe it as well. (The lockfile here is weird with the -0 & -1 matching)

what i did is to bump the versions in package.json and then run yarn

Ahh, yes, that is the issue - they are in multiple places in the root and in the workspace level (not actually enough, most pages do still pull in transient deps).

jacogr · 2021-12-22T08:35:01Z

Just merge in master and we should be good to go :)

jacogr

Thank you very much, this is great!

polkadot-js-bot · 2021-12-24T11:23:46Z

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.

add basic storage deposit input

abde497

statictype changed the title ~~Add storage deposit support~~ Add storage deposit support in apps Dec 10, 2021

statictype added 3 commits December 11, 2021 20:36

prefill input with free balance

0f488df

make value zeroable

2b9f212

changes to deploy modal

c6259de

athei reviewed Dec 12, 2021

View reviewed changes

packages/page-contracts/src/Codes/Upload.tsx Outdated Show resolved Hide resolved

statictype added 2 commits December 12, 2021 16:06

support older nodes

2d293fe

storage deposit support in call

799c54a

statictype mentioned this pull request Dec 13, 2021

Add storage deposit support polkadot-js/api#4304

Merged

statictype added 4 commits December 14, 2021 02:37

remove input and pass null

5ec21b0

remove unused

14b6082

properly check if value input shoudl be zeroable

fb13de8

remove unused

b3ad01d

athei reviewed Dec 16, 2021

View reviewed changes

hide input when constructor is not payable

a8ed790

statictype mentioned this pull request Dec 17, 2021

Storage deposit limit use-ink/contracts-ui#170

Closed

statictype added 3 commits December 21, 2021 19:14

Merge branch 'master' into ae-contract-storage-deposit

b6d2739

update api

377d75d

little cleanup

1eb31fa

jacogr reviewed Dec 22, 2021

View reviewed changes

jacogr mentioned this pull request Dec 22, 2021

Bump deps (dev, api, ui) #6724

Merged

remove unnecessary check

a89573e

Merge branch 'master' into ae-contract-storage-deposit

ab9b96c

jacogr approved these changes Dec 22, 2021

View reviewed changes

jacogr merged commit c2be422 into polkadot-js:master Dec 22, 2021

polkadot-js locked as resolved and limited conversation to collaborators Dec 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add storage deposit support in apps #6657

Add storage deposit support in apps #6657

statictype commented Dec 10, 2021 •

edited

Loading

statictype commented Dec 12, 2021 •

edited

Loading

jacogr commented Dec 13, 2021

athei Dec 16, 2021 •

edited

Loading

statictype Dec 16, 2021 •

edited

Loading

statictype Dec 16, 2021

athei Dec 16, 2021 •

edited

Loading

statictype Dec 16, 2021 •

edited

Loading

statictype commented Dec 17, 2021

jacogr commented Dec 21, 2021

statictype commented Dec 21, 2021

jacogr commented Dec 22, 2021

jacogr left a comment

jacogr Dec 22, 2021

statictype Dec 22, 2021

jacogr Dec 22, 2021 •

edited

Loading

jacogr Dec 22, 2021

statictype Dec 22, 2021

jacogr Dec 22, 2021

jacogr commented Dec 22, 2021

jacogr left a comment

polkadot-js-bot commented Dec 24, 2021

		isZeroable={hasStorageDeposit}
		isZeroable={contractAbi.constructors[constructorIndex].isPayable}

Add storage deposit support in apps #6657

Add storage deposit support in apps #6657

Conversation

statictype commented Dec 10, 2021 • edited Loading

statictype commented Dec 12, 2021 • edited Loading

jacogr commented Dec 13, 2021

athei Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

statictype Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

statictype Dec 16, 2021

Choose a reason for hiding this comment

athei Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

statictype Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

statictype commented Dec 17, 2021

jacogr commented Dec 21, 2021

statictype commented Dec 21, 2021

jacogr commented Dec 22, 2021

jacogr left a comment

Choose a reason for hiding this comment

jacogr Dec 22, 2021

Choose a reason for hiding this comment

statictype Dec 22, 2021

Choose a reason for hiding this comment

jacogr Dec 22, 2021 • edited Loading

Choose a reason for hiding this comment

jacogr Dec 22, 2021

Choose a reason for hiding this comment

statictype Dec 22, 2021

Choose a reason for hiding this comment

jacogr Dec 22, 2021

Choose a reason for hiding this comment

jacogr commented Dec 22, 2021

jacogr left a comment

Choose a reason for hiding this comment

polkadot-js-bot commented Dec 24, 2021

statictype commented Dec 10, 2021 •

edited

Loading

statictype commented Dec 12, 2021 •

edited

Loading

athei Dec 16, 2021 •

edited

Loading

statictype Dec 16, 2021 •

edited

Loading

athei Dec 16, 2021 •

edited

Loading

statictype Dec 16, 2021 •

edited

Loading

jacogr Dec 22, 2021 •

edited

Loading