Skip to content

Update contract' gas to Weight type #5324

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.

Sign up for GitHub

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

Merged
jacogr merged 13 commits into from
Nov 13, 2022
Merged

Update contract' gas to Weight type #5324

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
70087b1
Update contract' gas to Weight type
jasl Nov 8, 2022
05093af
Make Contracts Runtime Calls support WeightV2
jasl Nov 9, 2022
78258e6
fix gasConsumed & gasConsumed type
jasl Nov 9, 2022
4b0bcb7
Regenerate contracts types
jasl Nov 9, 2022
7647400
Correct ContractCallOutcome
jasl Nov 9, 2022
4668785
`gasLimit` are optional
jasl Nov 10, 2022
1da7ffa
Apply suggestion
jasl Nov 10, 2022
13e3828
Add ContractCallRequestV1 and specify old rpc use that
jasl Nov 11, 2022
5c0e8e1
Add back getGas
jasl Nov 11, 2022
c9f605d
Port jg-ContractsApi-v2 (#5330)
jasl Nov 11, 2022
5401b8a
Run yarn build:metadata
jasl Nov 11, 2022
277b7d8
There is no 'ContractCallRequestV1', only 'ContractCallRequest'
jasl Nov 11, 2022
bcddda6
api.tx.contracts.call.meta no version field, _isOldWeight still work?
jasl Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
api.tx.contracts.call.meta no version field, _isOldWeight still work?
  • Loading branch information
@jasl
jasl committed Nov 11, 2022
commit bcddda6548be74b038c36db8af234639b0ebcecc
5 changes: 3 additions & 2 deletions packages/api-contract/src/base/Contract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ export class Contract<ApiType extends ApiTypes> extends Base<ApiType> {
origin,
this.address,
value,
this.api.tx.contracts.call.meta.version === 1
? this.#getGas(gasLimit, true).v1Weight
this._isOldWeight
Copy link
Member

@jacogr jacogr Nov 11, 2022

Choose a reason for hiding this comment

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

We should not remove the old getGas here. It would still be applicable.

Rather when we detect v2 runtime (or rather v1 runtime) we swap to pushing through the old weight, not the option. So the existing will stay as-is for v1, but for v2 we will do the v2 weight thing.

Copy link
Contributor Author

@jasl jasl Nov 11, 2022

Choose a reason for hiding this comment

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

Sorry I don't understand your means, do you mean I need to revert this part change?

Copy link
Member

@jacogr jacogr Nov 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. It should rather be adapted to al check for the new stuff. You can do this by detecting which version is used, i.e something like -

...
value,
this.api.tx.contracts.call.meta.version === 1
  ? this.#getGas(gasLimit, true).v1Weight
  : this.#getGas(gasLimit, true).v2Weight
...

This is important since the getGas also allows the developer to pass a number - since we don't like breaking compat, we have not changed the signatures. And we have to ensure that the behviour on anything old stays exactly the same.

Copy link
Contributor Author

@jasl jasl Nov 11, 2022

Choose a reason for hiding this comment

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

Changed as your suggest

Copy link
Contributor Author

@jasl jasl Nov 11, 2022

Choose a reason for hiding this comment

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

packages/api-contract/src/base/Contract.ts:144:43 - error TS2339: Property 'version' does not exist on type 'FunctionMetadataLatest'.

144           this.api.tx.contracts.call.meta.version === 1

I'll dig

Copy link
Contributor Author

@jasl jasl Nov 11, 2022

Choose a reason for hiding this comment

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

seems FunctionMetadataLatest doesn't have version field?

Copy link
Contributor Author

@jasl jasl Nov 11, 2022

Choose a reason for hiding this comment

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

@jacogr I still don't got this one

// jiggle v1 weights, metadata points to latest
? this.#getGas(gasLimit, true).v1Weight as unknown as WeightAll['v2Weight']
: this.#getGas(gasLimit, true).v2Weight,
storageDepositLimit,
message.toU8a(params)
Expand Down