Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add ERC721Votes for NFT-based governance #2944
Add ERC721Votes for NFT-based governance #2944
Changes from 54 commits
abeaf5c
798baa0
4f5a44d
13d4a91
6ae58d3
c8abe20
a5d52d9
a2ec66b
d8147d4
2b03bf9
c987124
69c6bb1
339aa31
1cabd29
ec5ea0e
5f15d46
82b1565
8e3feae
d3f3502
1b95cc8
9335d04
c88fd74
b2c0568
f0dc612
88edb90
a0a26ce
0991757
9c3f9fd
0b0406e
4d3ca3a
d71fb49
dd4a695
2d834df
a207eb5
052d4a1
adbbbfb
4899be8
1f85057
3d0f1e4
9a3f2a1
f3d8453
ec047ac
1179e90
913946b
e999d78
3c63a1d
b42d0ab
dc0ec55
9d0f7fe
9efd8c1
87cf640
b135af5
510c373
f58c91e
8663d84
6d69091
72c887d
b94e475
a7397ba
6ed1ccd
718274a
e341b83
130664e
5c9e993
1ad617a
13d3b7c
f9fa57a
af0d1a5
3ccfdd9
05e8cee
b9c8e7b
068da60
e350d15
c5f44f7
7a9228f
49a378f
22c672e
514e0aa
074430d
a9d6cff
05ed326
3eb6d74
d8ff5cc
367f9bb
cca8ff2
1e0f543
84d97db
8147461
c914c84
2a45f36
deb2b1e
6fe31e9
51ade3f
3aec0a7
41aa303
4d1912b
2524a87
1e46d25
7d4c5fe
44df89d
55213f5
054c78f
92936c8
c7c9b29
d05cb0a
8582932
b57a3dc
02115ae
abc90f9
51002f9
54d80db
acd9a00
7f5a8f4
c919980
5a10153
792ce18
71301aa
9c39e32
a3396c5
3aef1f9
9784e04
1a3dc28
e37f4bf
f275406
6a9caf0
5a58fc2
e07dfa6
0bc0ab5
61523ed
f82873e
4f0f39e
acc4aa7
82e313b
e55511a
9eaa01d
29b0e16
3c49d6c
d344647
d687e09
101a82c
5201c96
4f9cc0a
4175ade
c8e1c94
de45b91
6af56a9
9079397
6eb60da
29e7a02
9a26c4d
af23b86
9b4d4cf
aa3c0db
2a3e090
e3730e1
731586f
9dad235
ae4d235
897938f
c0cab85
3b031da
9829c08
ada5c98
c730f4d
98a6618
ef43fb2
ec7e892
b813f9c
09da50d
e0e8d74
fb2bd07
d7f1dda
5506617
25eadd0
424af50
b2b7f5b
ef252c9
83eeac5
176d166
bb32fba
1d0c1ea
3e14830
f7a7b51
5740f3c
5704b3b
be216d0
c37502f
a28a4b2
9ea08be
6bcff2b
73bb657
3ffdb76
b6cbe1d
099fa20
f054441
0c8b601
7de147c
d852c08
5efd9a2
052f064
76d47c7
499559a
eb6304b
e5fecaf
9addc4f
f536b1b
b8639e7
f5f6aab
87bd7b2
93a2079
829f074
8fdb1e0
dfe225c
ee10185
1cca7be
c173749
f2cccc3
2f8cf7b
628567c
edf795b
7f90b88
7cda9a3
a82fc9c
b171720
7e134d8
1197d84
c60d9b5
81972f7
4773334
7ead55f
f099426
c4d3dfd
43abcee
cd3ab17
c349618
ebd3082
ce1df1f
5cb991b
37c9e73
d556b27
94d63e0
547cba4
90ca1e2
9d705ce
8e74446
d6ae076
92537f3
b306e61
53847b4
bd6ec00
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
AFAIK, this module produces the exact same result as GovernorVotesERC20, because the
getPastVotes
method is shared. Maybe we should not have 2 identical modulesThere 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.
Wouldnt it serve as a guide for users trying to implment Governance with ERCVotes? Or should we include some documentation somewhere instead?
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.
I hadn't realized before that this module was exactly the same as
GovernorVotes
.It's better to avoid the duplication. As a guide for users, we should change
GovernorVotes
so that it's clear that it is used with both:ERC20Votes
so that it's clear that it can be used withERC721Votes
as well. We should introduce anIVotes
interface that is implemented by bothERC20Votes
andERC721Votes
, and use it inGovernorVotes
in place ofERC20Votes
.