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

Breadboard: Checkout across rounds #1796

Closed
meglister opened this issue May 15, 2023 · 6 comments
Closed

Breadboard: Checkout across rounds #1796

meglister opened this issue May 15, 2023 · 6 comments
Assignees

Comments

@meglister
Copy link
Member

meglister commented May 15, 2023

Research Objectives

  • Become super familiar with this part of the code base for future work
  • Get all projects across all active rounds into one cart
  • Get all projects across all active rounds into one transaction (per chain)

Output: demo for the Lunarpunk team by end of the week

Out of scope: cross-chain checkout

@hmrtn hmrtn self-assigned this May 15, 2023
@meglister
Copy link
Member Author

Note - could be worth looking up twitter thread or old implementation on cGrants

@boudra
Copy link
Contributor

boudra commented May 16, 2023

maybe I am misunderstanding the task but couldn't we add each donation in an array in localStorage with this shape:

{
   applicationId: string;
   roundId: string;
   chainId: number;
}

so it would have projects mixed up from all rounds

and then in the checkout page you could group projects by round and network and do separate transactions for each network

@meglister
Copy link
Member Author

Attaching the sketch we discussed in discord

Image

@hmrtn
Copy link
Contributor

hmrtn commented May 17, 2023

@hmrtn hmrtn closed this as completed May 17, 2023
@hmrtn
Copy link
Contributor

hmrtn commented May 18, 2023

Summary of the spike: To improve the UX, supporting x-round checkout is necessary. The current idea of solving this is to implement a MultiCheckout intermediary contract. This will require implementing the MultiCheckout contract and some changes to the current contracts. One idea is to change the vote call in the round implementation: https://github.com/allo-protocol/contracts/blob/34ca9a82a050601e32d82f2d79a353fde732964f/contracts/round/RoundImplementation.sol#L441
Such that the msg.sender is replaced with with the tx.origin. However this could introduce security problems - see: https://discord.com/channels/562828676480237578/1108094112617484398/1108301809291898942
Another proposal from Kurt suggests making changing within the Grant Stack rather than the protocol itself, where instead of relying on the suggested changes above, we introduce a list of trusted contracts that are allowed to use the MultiCheckout contract. We would only need to add a new subscription to the contracts in order to index the data. An issue with this might be implementations built on the protocol, as they can have different ways to count votes. So changes here may not be ideal.
Also, we could simply add a new field in the event emission of a vote to include a tx.origin. that way we still know the caller, and its origin. But we should investigate if this is the ideal path, or just a “hack” for Grants Stack. We do not want to silo the protocol to Gitcoin only.

See this thread for full context: https://discord.com/channels/562828676480237578/1108094112617484398

@thelostone-mc
Copy link
Member

Personally, I'd lean towards emitting tx.origin along with the event.
This is just surfacing more information and the client (grants-stack) can use this in their calculations.

First option -> introduced security issues
Second option -> seems like a lot of overhead
Third option -> helps us solve the issue with min complexity and does not compromise on security

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

No branches or pull requests

4 participants