Skip to content
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

CIP-???? | Wallet api.signTxs() method #443

Closed
wants to merge 2 commits into from

Conversation

MartinSchere
Copy link

@MartinSchere MartinSchere commented Jan 19, 2023

Motivation

Currently, there is no way to sign multiple transactions in bulk, and the experience of signing a chain of transactions is suboptimal. This is because the standard signTx method does not provide the wallet with a way to resolve a (yet) nonexistent input.

Description

We propose the addition of a signTxs method that enables wallets to create an array of interconnected transactions and sign them all at once. This new method improves the signing experience by allowing for bulk transaction signing.

@L-as
Copy link
Contributor

L-as commented Jan 19, 2023

Transactions cannot share the same inputs (unless these are reference inputs), so input selection has to be performed carefully.

Some times you send off mutually exclusive transactions of which only one can succeed. This is very useful for that. Hence, I request that you remove this artificial limitation.

@MartinSchere
Copy link
Author

@L-as Edited!

Copy link
Contributor

@L-as L-as left a comment

Choose a reason for hiding this comment

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

LGTM but why all the formatting changes?

@MartinSchere
Copy link
Author

My formatter I guess, we are missing a .editorconfig in this repo (cc @KtorZ)

@KtorZ
Copy link
Member

KtorZ commented Jan 20, 2023

I have no particular issue with this change and the motivation behind it. Yet I have an issue with the lack of versioning / extension mechanism on CIP-0030.

CIP-0030 has been effectively implement by many wallet providers and dapps alike as of today. So altering the current interface makes it hard to track what version wallet providers are supporting and what functionalities can DApps actually expect.

@rphair
Copy link
Collaborator

rphair commented Jan 20, 2023

@KtorZ - didn't we discuss at one of the CIP meetings, around the time CIP-0030 was merged, some kind of versioning system that would allow wallets to indicate or query support for certain features? If I'm not simply remembering incorrectly, whatever happened to that?

@Scitz0
Copy link
Contributor

Scitz0 commented Jan 20, 2023

@KtorZ - didn't we discuss at one of the CIP meetings, around the time CIP-0030 was merged, some kind of versioning system that would allow wallets to indicate or query support for certain features? If I'm not simply remembering incorrectly, whatever happened to that?

Further discussion about the extension/versioning of CIP-30 can be had here:
#446

@KtorZ KtorZ added the Category: Wallets Proposals belonging to the 'Wallets' category. label Mar 18, 2023
@Scitz0
Copy link
Contributor

Scitz0 commented Aug 17, 2023

Can we get the editors group to look at this one again?

We are about to add this extension to CIP-30 in Eternl and I want to do it the "proper" way by having it added as an extension to CIP-30. That would mean giving this CIP its own CIP number. Dapps would then request access to it the established way by providing extension object in enable call. It would be up to the wallet to decide if this should yield an additional request for access in addition to cip30 but from our perspective it wouldn't and if cip30 access is granted, it would automatically allow signTxs() as well.

Any changes to the text needed to finalize this CIP?
If so, please specify what.

@rphair
Copy link
Collaborator

rphair commented Aug 17, 2023

@Scitz0 so we can all agree on exactly what is most urgently needed, please see the current discussion about namespacing (#577) and make a comment there (cc @Ryun1). It could be what what you are looking for will be accomplished by naming your extension points uniquely: though the current plan is to group these into a cipXX type namespace & so indeed this would likely need a unique CIP number.

@Scitz0
Copy link
Contributor

Scitz0 commented Aug 17, 2023

I have looked at that namespacing PR yes and would make this available under api.cipXX.signTxs() to align with that PR.

@rphair
Copy link
Collaborator

rphair commented Aug 17, 2023

@Ryun1 @KtorZ @MartinSchere I assume this means this PR would have to be re-cast as an independent CIP so it can be assigned its own CIP number?

@Ryun1
Copy link
Collaborator

Ryun1 commented Aug 17, 2023

I assume this means this PR would have to be re-cast as an independent CIP so it can be assigned its own CIP number?

Yes, this PR should be changed to be a new CIP 🙏

@Scitz0
Copy link
Contributor

Scitz0 commented Aug 17, 2023

I will whip up a new CIP PR based on content from this one later tonight or tomorrow.

@rphair rphair changed the title CIP-0030 | Add api.signTxs() method CIP-???? | Wallet API api.signTxs() method Aug 17, 2023
@rphair rphair changed the title CIP-???? | Wallet API api.signTxs() method CIP-???? | Wallet api.signTxs() method Aug 17, 2023
Scitz0 added a commit to Scitz0/CIPs that referenced this pull request Sep 3, 2023
Replaces previous PR cardano-foundation#443 by adopting CIP-30 extension framework as well as to-be-merged CIP-30 namespace PR cardano-foundation#577.
@rphair
Copy link
Collaborator

rphair commented Sep 4, 2023

Superseded by #587 with the same purpose & scope.

@rphair rphair closed this Sep 4, 2023
rphair added a commit that referenced this pull request May 28, 2024
* CIP-???? | CIP-30 ext: Bulk transaction signing

Replaces previous PR #443 by adopting CIP-30 extension framework as well as to-be-merged CIP-30 namespace PR #577.

* update api call to use cip namespace

* add checkboxes for items we're not sure are done yet

* word accuracy + grammar

* Mark criteria as completed

* include *all* prior GitHub discussion (2 PRs)

Co-authored-by: Ryan Williams <[email protected]>

* Update according to #590

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Update CIP-XXXX/README.md

Co-authored-by: Ryan Williams <[email protected]>

* Addresses mentioned comments

* change delimiter character as per #594

* title delimiter changed to - as per #594 update

* assign CIP number 103

* Updates according to assigned CIP number

* Updates according to #601

* Update CIP-0103/README.md

Co-authored-by: Ryan <[email protected]>

* Update CIP-0103/README.md

Co-authored-by: Ryan <[email protected]>

* Update CIP-0103/README.md

Co-authored-by: Ryan <[email protected]>

---------

Co-authored-by: Robert Phair <[email protected]>
Co-authored-by: Ryan Williams <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Wallets Proposals belonging to the 'Wallets' category.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants