-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: Add methods multisig.{asMulti, approveAsMulti, cancelAsMulti} #52
Conversation
packages/txwrapper-substrate/src/methods/multisig/asMulti.spec.ts
Outdated
Show resolved
Hide resolved
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.
LGTM
Co-authored-by: Tarik Gul <47201679+TarikGul@users.noreply.github.com>
packages/txwrapper-substrate/src/methods/multisig/approveAsMulti.ts
Outdated
Show resolved
Hide resolved
packages/txwrapper-substrate/src/methods/multisig/approveAsMulti.ts
Outdated
Show resolved
Hide resolved
packages/txwrapper-substrate/src/methods/multisig/approveAsMulti.ts
Outdated
Show resolved
Hide resolved
packages/txwrapper-substrate/src/methods/multisig/approveAsMulti.ts
Outdated
Show resolved
Hide resolved
* | ||
* NOTE: If this is the final approval, you will want to use `as_multi` instead. | ||
* | ||
* @param args - Arguments specific to this method. |
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.
"Arguments specific to this method." isn't very helpful. How about "A MultiSigApproveAsMulti
containing the arguments for the approval call."?
Also, I'm a bit confused by the difference between argument and options here. The MultiSigApproveAsMulti
seem to contain things I'd consider options, e.g. threshold
and maxweight
.
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.
args
and options
here are specific to the txwrapper
API and these docs comments are universal to all txwrapper methods so I am hesitant to change thing since it would destroy consistency.
args
will always be an interface with the arguments specific to the method. In this case that interface is MultiSigApproveAsMulti
, which is shown in the function signature.
options
here does not have anything to do with the call itself. Instead the options
in txwrapper methods are the metadata
and polkadot-js TypeRegistry
and are used any time you need to construct or decode a payload at any step in the process transaction construction process.
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.
Ok I see. Ty for explaining. :)
*/ | ||
call: string; | ||
/** | ||
* Wether or not to store the call. |
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.
dq: store where?
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.
See updated comment
Co-authored-by: David <dvdplm@gmail.com>
…er-core into zeke-multisig
* Wether or not to store the call in the pallet storage item `Calls`. Storing the call | ||
* is normally only useful if this is the first approval, threshold > 1, and you | ||
* want the call stored on chain so others can see. The call will always be | ||
* removed from storage once the call is executed. |
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.
👍
Adds the following methods from the
multisig
pallet:asMulti
approveAsMulti
cancelAsMulti
Additionally, bumps polkadodt-js related deps and makes relevant adjustments.