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

Contract Self-Signed gasless poll creation & voting #9

Merged
merged 21 commits into from
Aug 21, 2023

Conversation

CedarMist
Copy link
Member

@CedarMist CedarMist commented Jul 16, 2023

This hoists my previous gasless voting PR onto the new design + fixes lots of UX things which made it tedious to use.

Deploy with the gasless voting addition:

$ pnpm hardhat deploy --network sapphire-localnet --gasless

Uses EIP-712 for both poll creation and poll voting. Aside from creating, voting and closing polls there are no other interjections from metamask. Users don't need to have gas on Sapphire to perform these operations.

image

TODO:

@CedarMist CedarMist marked this pull request as ready for review July 16, 2023 02:34
@CedarMist CedarMist requested a review from matevz July 16, 2023 03:28
@CedarMist CedarMist self-assigned this Jul 16, 2023
matevz
matevz previously requested changes Jul 16, 2023
Copy link
Member

@matevz matevz left a comment

Choose a reason for hiding this comment

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

First pass, I didn't test it yet, will do it later.

I suggest we have demo-voting supporting all three mechanisms:

  • payable votes (current master branch)
  • GSN votes
  • gasless votes (we should come up with a fancy Oasis name for it?)

That being said I would not merge this until we cleanup the backend/contracts:

  • EIP155Signer.sol should become part of @oasisprotocol/sapphire-contracts
  • IERC165 should just be included from openzeppelin?
  • RLPWriter should be part of @oasisprotocol/sapphire-contracts
  • DAOv1 should be an interface with three implementations mentioned above

If we don't make it for EthCC, it's even fine to have it in a separate PRs for now.

Thoughts?


import { useDAOv1 } from '../contracts';
import { useDAOv1, useGaslessVoting, useUnwrappedDAOv1, useUnwrappedGaslessVoting } from '../contracts';
Copy link
Member

Choose a reason for hiding this comment

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

Could you move this to a separate PR please? I would need that for my GSN voting as well and let's merge it separately ;)

@@ -25,10 +25,72 @@ export function useDAOv1(): ComputedRef<DAOv1> {
});
}

export function useUnwrappedDAOv1(): ComputedRef<DAOv1> {
Copy link
Member

Choose a reason for hiding this comment

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

Could you move useUnwrappedDAO and useUnwrappedPollACLv1 to a separate PR please? I would need that for my GSN voting as well and let's merge it separately ;)

@CedarMist
Copy link
Member Author

CedarMist commented Jul 16, 2023 via email

@CedarMist
Copy link
Member Author

CedarMist commented Jul 16, 2023

DAOv1 should be an interface with three implementations mentioned above

There is no need for DAOv1 to be an interface with multiple implementations, unless we follow OpenZeppelin Governance interfaces or similar.

GSN implementation can use the same pattern as I did with GaslessVoting.

  • DAOv1 is assigned an optional 'proxy voter' contract, aka a trusted verifier which can override msg.sender
  • GSN contract interfaces with DAOv1 via proxyVote interface
  • Client detects which proxy voter implementation is being used, via ERC-165

@matevz matevz force-pushed the CedarMist/feature/gasless-voting-2 branch from 7afe0aa to a2cbdcc Compare July 20, 2023 14:30
@matevz matevz force-pushed the CedarMist/feature/gasless-voting-2 branch from fd184b6 to 83881f9 Compare August 17, 2023 15:27
@matevz matevz force-pushed the CedarMist/feature/gasless-voting-2 branch from 83881f9 to 550a21f Compare August 17, 2023 15:34
@CedarMist CedarMist dismissed matevz’s stale review August 18, 2023 17:56

Need to dismiss review after you took over

@CedarMist CedarMist requested a review from matevz August 18, 2023 17:56
@CedarMist
Copy link
Member Author

I can't merge this because it's my branch. You will need to approve it.

@matevz matevz force-pushed the CedarMist/feature/gasless-voting-2 branch from 0ff9572 to 6272f22 Compare August 21, 2023 10:37
@matevz matevz enabled auto-merge August 21, 2023 10:38
@matevz matevz merged commit 7baed6e into master Aug 21, 2023
1 check passed
@matevz matevz deleted the CedarMist/feature/gasless-voting-2 branch August 21, 2023 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants