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

ARC-55 On-Chain Msig Apps #254

Merged
merged 7 commits into from
Dec 8, 2023
Merged

Conversation

nullun
Copy link
Contributor

@nullun nullun commented Oct 23, 2023

This is an initial draft for the on-chain multisignature application proposal. (If someone can think of a good name for this, please share)

The TL;DR is it's a smart contract design you and your friends can use to help with signing multisig transactions.

The smart contract itself does not perform any multsignature operations, but rather it facilitates the distribution of the unsigned transactions between the multisignature constituents, and allowing them to store their signature on-chain.

This should make using multisig accounts and signing transactions much faster and simpler.

@SudoWeezy SudoWeezy changed the title On-Chain Msig Apps ARC-55 On-Chain Msig Apps Oct 25, 2023
@SudoWeezy
Copy link
Collaborator

We should also add the arc55_ prefix to each method

ARCs/arc-0055.md Outdated
@@ -235,7 +235,7 @@ Having individual deployments for different groups rather than a single instance

## Reference Implementation

A TEALScript reference implementation is available at <a href="https://github.com/nullun/arc55-msig-app">github.com/nullun/arc55-msig-app</a> or an older TEAL implementation at <a href="https://github.com/nullun/MsigShare">github.com/nullun/MsigShare</a> with a user interface at <a href="https://github.com/nullun/MsigShareUI">github.com/nullun/MsigShareUI</a>. It is encouraged for others to implement this standard in their preferred smart contract language of choice and even extend the capabilities whilst adhering to the provided ABI specification.
A TEALScript reference implementation of is available at <a href="https://github.com/nullun/arc55-msig-app">`github.com/nullun/arc55-msig-app`</a> or an older TEAL implementation at <a href="https://github.com/nullun/MsigShare">`github.com/nullun/MsigShare`</a> with a user interface at <a href="https://github.com/nullun/MsigShareUI">`github.com/nullun/MsigShareUI`</a>. It is encouraged for others to implement this standard in their preferred smart contract language of choice and even extend the capabilities whilst adhering to the provided ABI specification.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

of what?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The linting check doesn't allow you to write arc55, it should be ARC-55, so your link raised an error.
The trick to allow it is to put links in code format.

@HashMapsData2Value
Copy link

HashMapsData2Value commented Dec 4, 2023

This is specifically for Algorand's native ed25519-based msig. But with AVM v10 BLS msigs will be possible as well on-chain in lsigs, and whose shape might be nice to define in its own ARC. Perhaps the name/title of this arc can be made more specific?

@SudoWeezy SudoWeezy merged commit ec529a0 into algorandfoundation:main Dec 8, 2023
5 checks passed
@k13n
Copy link
Contributor

k13n commented Dec 9, 2023

@nullun, thanks a lot for this proposal, I'm really looking forward to interoperable multisigs across wallets. Here some feedback:

  • Wallet providers need a way to discover pending signature requests for a user. For mobile wallets, it'd additionally be useful if a server could easily detect pending signature requests and send out push notifications to its users. In the current design, each msig has its dedicated application, hence a server would have to (a) find out what applications are ARC-55 msig apps and (b) monitor all those apps to see if there are any pending signature requests. There could be a registry where all msig apps opt in to announce themselves, which would address point (a), but not really point (b). @joe-p suggested turning this app into a monolith, which would address both points but require significant changes to the code, I guess.

  • Currently, only the admin account (i.e., the creator account) can add transactions and ask for them to be signed. I think every signer in a multisig should be treated equally and have the ability to propose and sign transactions.

  • In the current design, it seems that only one transaction group can be added and signed at the same time. I think it'd be nice if there could be multiple pending transaction groups at the same time. There can be cases where multiple signers create transactions at the same time, or where one signer creates multiple transaction groups at the same time.

  • Should the app be updateable? Assuming a wallet provider indexes all the ARC-55 msig apps and one deployed app suddenly changes it may break all kinds of code. I don't really see the value in having an updateable contract for the msig application, but I'm happy to be convinced otherwise:)

  • Why can the threshold in the app be changed? Is an intended use case that the same msig application is reused for different msig configurations?

  • Adding to the previous point, it seems accounts can also be changed at any time. This means, the msig configuration can change over time. That makes the application very flexible, but more difficult to deal with for wallet providers. If wallets track msig applications on their backend, they'd also have to keep track of changes to the accounts/thresholds in the applications.

  • What kind of limitations are there on the transactions that can be submitted? I'm wondering if there are any transaction groups that are valid but that could not be submitted through ARC-55. I'm thinking of transaction groups that create an app with lots of extra program pages and hence get fairly big. Currently, hardware ledger devices cannot sign create-app txns that are too big, which is fairly annoying in my opinion. The next most secure way of signing them would be with a multisig accounts, hence my question about what kinds of limitations there are.

  • I really appreciate that you gave a breakdown of the costs that occur for the various operations (this is super useful for a wallet to show to its users). Could there be further details about whether these costs are recoverable or not and when they're recoverable? What I mean is that certain operations add to the minimum balance (e.g., adding a transaction) and it would be nice to explicitly state when the minimum balance is reduced again (e.g., when a transaction is removed again from the app).

  • Speaking of minimum balance, who pays for the app's minimum balance requirements for the boxes? Probably the user that posts the transactions?

I know this is a lot and not everything may make it into the final ARC, but this just came to my mind when I went through the ARC. In any case, great work!

@nullun
Copy link
Contributor Author

nullun commented Dec 9, 2023

Hey @k13n, thank you for this! I appreciate the thorough review and feedback. I'll provide a response to each section below.

  • Wallet providers need a way to discover pending signature requests for a user. For mobile wallets, it'd additionally be useful if a server could easily detect pending signature requests and send out push notifications to its users. In the current design, each msig has its dedicated application, hence a server would have to (a) find out what applications are ARC-55 msig apps and (b) monitor all those apps to see if there are any pending signature requests. There could be a registry where all msig apps opt in to announce themselves, which would address point (a), but not really point (b). @joe-p suggested turning this app into a monolith, which would address both points but require significant changes to the code, I guess.

@joe-p and I discussed this a little yesterday after the meeting. My main goal with this ARC is to create a common interface that people can use either as it is (using the reference contract), modify the internal logic to meet their own needs, or even implement into unrelated contracts which themselves would now support ARC55 calling conventions. I strongly dislike the idea of us deploying a single version that we deem fit for everyone, just because it would make a centralised notification system easier.

Whilst I do agree that a push notification would be nice, I don't feel it's necessary for the implementation of this ARC. Primarily this ARC attempts to address the struggles with sharing transaction and signature data, and assumes the participants of a multisig are already well enough connected, that sending a text/slack/discord message doesn't need improving.

I imagined this working as followed:

  1. A group of people agree they need a multisig
  2. One of the members deploys the msig app (paying the MBR costs themselves)
  3. The msig appID is then shared with the rest of the party, so they can each add it to their ARC55 supported wallet/app they use
  4. When a transaction is added, either a notification is shown to the user because their wallet/app is listening locally to the state of the app which they've added. Or the person who uploaded the transaction tells the group via any existing communication channels.
  • Currently, only the admin account (i.e., the creator account) can add transactions and ask for them to be signed. I think every signer in a multisig should be treated equally and have the ability to propose and sign transactions.

I fully agree with this after hearing the feedback. I'll make my reference implementation default to allowing anyone who is part of the multisig able to add transactions. Although touching on a point I made above, if someone decides their requirements only warrant one account to modify it, they should be fully allowed to do so as long as the interface remains the same. If we deployed one single implementation which all wallets look at, we now lose out on reusing the same interfaces for something that should still adhere to the standard and work (albeit gracefully handling it when someone who doesn't have permission to upload a transaction attempts to).

  • In the current design, it seems that only one transaction group can be added and signed at the same time. I think it'd be nice if there could be multiple pending transaction groups at the same time. There can be cases where multiple signers create transactions at the same time, or where one signer creates multiple transaction groups at the same time.

Yeah, this is definitely something I hadn't thought about, purely because I've never personally experienced or seen a multisig party do it. There isn't actually anything stopping you from uploading 16 individual transactions, or even 2 groups of 8 transactions, etc and then displaying them as separate pending groups/transactions. However the reference definitely has a hardcoded limit of 16 transactions being stored.

I like this idea a lot though. I'll take a look and see how much effort would be involved reworking the box storage. Off the top of my head, I'm thinking we could add an incrementing nonce for every transaction/group that gets uploaded. So the box names could be something like 0txn0, 0txn1, 0txn2 for the first group, then 1txn0, 1txn2, for the next upload. Although this would require signatures to also be placed in boxes, since they could now provide more than 16 signatures at any one time.

This would definitely require some stronger "garbage collection" incentives/wording though, to encourage users to delete historic transactions.

  • Should the app be updateable? Assuming a wallet provider indexes all the ARC-55 msig apps and one deployed app suddenly changes it may break all kinds of code. I don't really see the value in having an updateable contract for the msig application, but I'm happy to be convinced otherwise:)

I don't think there should be an update feature defined in the ARC. It should either be deploy when you need one, or destroy when it's no longer required. But again, if someone wants to include an update feature for their own version, that's on them. As long as the ARC55 interface remains the same then they can use any ARC55-supported wallet/app to use it.

  • Why can the threshold in the app be changed? Is an intended use case that the same msig application is reused for different msig configurations?
  • Adding to the previous point, it seems accounts can also be changed at any time. This means, the msig configuration can change over time. That makes the application very flexible, but more difficult to deal with for wallet providers. If wallets track msig applications on their backend, they'd also have to keep track of changes to the accounts/thresholds in the applications.

Great points! When I was originally testing this, it was easier for me to just update the thresholds and accounts on an existing deployment. But I could very well see these parameters as something you define during the deployment and are then locked. This way you'd have to deploy a new instance if you want different threshold/accounts? I think this makes the ARC a lot simpler to understand and clearer.

  • What kind of limitations are there on the transactions that can be submitted? I'm wondering if there are any transaction groups that are valid but that could not be submitted through ARC-55. I'm thinking of transaction groups that create an app with lots of extra program pages and hence get fairly big. Currently, hardware ledger devices cannot sign create-app txns that are too big, which is fairly annoying in my opinion. The next most secure way of signing them would be with a multisig accounts, hence my question about what kinds of limitations there are.

Yes, this is absolutely the biggest "issue" in my opinion, but I decided for most multisig use-cases where it's just sending funds this isn't a blocker.

An example where a valid transaction wouldn't be possible in this ARC would be where the group contains transactions that are not from the multisig address, such as an atomic swap between two accounts (the multisig and any other account). The other account in this scenario would be unable to provide their signature to the msig app, unless they were the one who reconstructed and it after everyone else had provided their signatures, and then added their own.

A discussion I had internally lead us down using this same smart contract design, but for more generic situations, where anyone can provide their signature for a transaction. Allowing for true atomic swaps between different individuals, using just the chain as a transport layer. This however falls outside the scope of this ARC.

  • I really appreciate that you gave a breakdown of the costs that occur for the various operations (this is super useful for a wallet to show to its users). Could there be further details about whether these costs are recoverable or not and when they're recoverable? What I mean is that certain operations add to the minimum balance (e.g., adding a transaction) and it would be nice to explicitly state when the minimum balance is reduced again (e.g., when a transaction is removed again from the app).
  • Speaking of minimum balance, who pays for the app's minimum balance requirements for the boxes? Probably the user that posts the transactions?

Excellent idea. Whilst I'm changing who can add transactions, I'll make sure it's clear that the uploader of the transactions is responsible for adding the MBR to the application address. Although I don't see an elegant solution to recover this back to the them. Rather it's only recoverable by the creator upon removing all the boxes and destroying the msig app? Unless there's a nice simple alternative?

I know this is a lot and not everything may make it into the final ARC, but this just came to my mind when I went through the ARC. In any case, great work!

Thank you so much again for taking the time to go through it! I'm sorry my response is equally as long. Please let me know if there's anything you strongly disagree with, we're definitely looking at this from different perspectives, so I need to be open to at least think more about them.

@k13n
Copy link
Contributor

k13n commented Dec 10, 2023

@joe-p and I discussed this a little yesterday after the meeting. My main goal with this ARC is to create a common interface that people can use either as it is (using the reference contract), modify the internal logic to meet their own needs, or even implement into unrelated contracts which themselves would now support ARC55 calling conventions. I strongly dislike the idea of us deploying a single version that we deem fit for everyone, just because it would make a centralised notification system easier.

Whilst I do agree that a push notification would be nice, I don't feel it's necessary for the implementation of this ARC. Primarily this ARC attempts to address the struggles with sharing transaction and signature data, and assumes the participants of a multisig are already well enough connected, that sending a text/slack/discord message doesn't need improving.

I imagined this working as followed:

  1. A group of people agree they need a multisig
  2. One of the members deploys the msig app (paying the MBR costs themselves)
  3. The msig appID is then shared with the rest of the party, so they can each add it to their ARC55 supported wallet/app they use
  4. When a transaction is added, either a notification is shown to the user because their wallet/app is listening locally to the state of the app which they've added. Or the person who uploaded the transaction tells the group via any existing communication channels.

Thanks for this context. I see the value in having a common set of calling conventions for msigs with lots of different implementations and a lot of flexibility, but this makes it really difficult to integrate smoothly and provide a good interface for.

It seems there are two issues:

  • App discovery: how can an integrator detect that an account is opted into a ARC-55 app? When a user imports an account into a wallet, I'd like to be able to tell that a particular app is compatible with ARC-55 and that I should be listening for transactions.
  • Transaction discovery: how can an integrator detect that there are pending signature requests? With the current approach it's easy to check in the frontend, but hard to check in the backend. From user feedback I can tell you that automatically detecting pending transactions and sending push notifications to users is a highly appreciated feature and I wouldn't want to lose the ability to do that.

It'd be great to hear @yigitguler's opinion on this and how they would potentially integrate ARC-55 in Pera Wallet.

I like this idea a lot though. I'll take a look and see how much effort would be involved reworking the box storage. Off the top of my head, I'm thinking we could add an incrementing nonce for every transaction/group that gets uploaded. So the box names could be something like 0txn0, 0txn1, 0txn2 for the first group, then 1txn0, 1txn2, for the next upload. Although this would require signatures to also be placed in boxes, since they could now provide more than 16 signatures at any one time.

This would definitely require some stronger "garbage collection" incentives/wording though, to encourage users to delete historic transactions.

Great, thanks for looking into this! I kind of liked the idea that the signatures are stored in the signer's local state, just because then the minimum balance requirement is already taken care of when the user opts in. But I think having boxes to support more than 16 transactions is important and makes sense. Perhaps simply whoever creates the transactions should make sure that the msig app has sufficient minimum balance for all signatures. But it'd be good for interoperability that there's a agreed upon way of doing it. If one integrator thinks the transaction creator should pay the min balance and another integrator thinks the signers should pay the min balance, we're going to have problems.

Yes, this is absolutely the biggest "issue" in my opinion, but I decided for most multisig use-cases where it's just sending funds this isn't a blocker.

An example where a valid transaction wouldn't be possible in this ARC would be where the group contains transactions that are not from the multisig address, such as an atomic swap between two accounts (the multisig and any other account). The other account in this scenario would be unable to provide their signature to the msig app, unless they were the one who reconstructed and it after everyone else had provided their signatures, and then added their own.

A discussion I had internally lead us down using this same smart contract design, but for more generic situations, where anyone can provide their signature for a transaction. Allowing for true atomic swaps between different individuals, using just the chain as a transport layer. This however falls outside the scope of this ARC.

I think it's okay that not every conceivable transaction group can be represented in ARC-55, but it'd be great to make these things explicit in the ARC such that it's very clear what is and what isn't possible.

@nullun
Copy link
Contributor Author

nullun commented Dec 10, 2023

Thanks for this context. I see the value in having a common set of calling conventions for msigs with lots of different implementations and a lot of flexibility, but this makes it really difficult to integrate smoothly and provide a good interface for.

It seems there are two issues:

* **App discovery**: how can an integrator detect that an account is opted into a ARC-55 app? When a user imports an account into a wallet, I'd like to be able to tell that a particular app is compatible with ARC-55 and that I should be listening for transactions.

* **Transaction discovery**: how can an integrator detect that there are pending signature requests? With the current approach it's easy to check in the frontend, but hard to check in the backend. From user feedback I can tell you that automatically detecting pending transactions and sending push notifications to users is a highly appreciated feature and I wouldn't want to lose the ability to do that.

It'd be great to hear @yigitguler's opinion on this and how they would potentially integrate ARC-55 in Pera Wallet.

Yeah, app discovery for specifically supported ARCs is definitely a wider issue than just ARC55. Any smart contract interface that's proposed that doesn't offer a single existing deployed solution will have the same difficulties. I recall a number of discussions in the past about trying to solve it. One that comes to mind is embedded the ARC number in the smart contract, for example appending intcblock 4 55 at the very end would indicate supporting ARC4 and ARC55 To my knowledge there hasn't been anything concrete proposed on this topic. I think I'd still personally prefer to be adding the appID manually myself though, rather than being sent notifications because someone is maliciously adding my account to a msig app that I have nothing to do with.

Transaction discovery is possibly easier to solve, with the inclusion of ARC-28: Algorand Event Log Spec. If each ARC55 implementation requires logging TransactionAdded(bytes) when a new transaction was added, and SignatureAdded(address), along with ThresholdMet(uint64) or something like this, then it just requires a single monitoring service to be keeping track of events in each block. Maybe a specific Conduit plugin could be made to specifically track every single event. I'm using this ARC for another contract I'm working on, and have been implementing a script that monitors the chain for events as they happen. I must point out the current issue related to this though.

Great, thanks for looking into this! I kind of liked the idea that the signatures are stored in the signer's local state, just because then the minimum balance requirement is already taken care of when the user opts in. But I think having boxes to support more than 16 transactions is important and makes sense. Perhaps simply whoever creates the transactions should make sure that the msig app has sufficient minimum balance for all signatures. But it'd be good for interoperability that there's a agreed upon way of doing it. If one integrator thinks the transaction creator should pay the min balance and another integrator thinks the signers should pay the min balance, we're going to have problems.

That's a good idea. I'll see which flow makes most sense and is easiest for integrators to use. Ideally you're not having to calculating the cost for each interaction, so one-and-done feels simpler.

I think it's okay that not every conceivable transaction group can be represented in ARC-55, but it'd be great to make these things explicit in the ARC such that it's very clear what is and what isn't possible.

100% I'll add this to the ARC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants