Implement OpenGov Precompiles #1885

4meta5 · 2022-10-20T18:55:33Z

MOON-1966

Follow ups

rust precompile unit tests
TS test with smart contracts
Add more constants, storage items, not included
Add events

…ing std librar

pallets/governance-origins/src/lib.rs

4meta5 · 2023-01-04T23:12:51Z

resolved

…stain in future version without breaking backwards compatibility

precompiles/conviction-voting/ConvictionVoting.sol

notlesh · 2023-01-17T17:45:06Z

precompiles/conviction-voting/src/lib.rs

+	/// * vote_amount: Balance locked for vote
+	/// * conviction: Conviction multiplier for length of vote lock
+	#[precompile::public("voteNo(uint32,uint256,uint8)")]
+	fn vote_no(


nit: this is very repetitive with vote_yes. I'm not sure why we made this two fns at the solidity interface level, but they don't have to be two fns here (less code duplication).

I think having two separate functions is fine (a boolean is less of an intuitive thing IMHO). But what I do agree with is that the code should be merged for both

We need to anticipate for the future vote with "abstain" making vote support 3 possibilities.

I agree with @girazoki, the best is to create a inner_vote function that takes as parameter the nature of the vote in order to factorize the code.

We need to anticipate for the future vote with "abstain" making vote support 3 possibilities.

The backwards compatible way is to add a new voteAbstain function when this is exposed.

I think having two separate functions is fine (a boolean is less of an intuitive thing IMHO). But what I do agree with is that the code should be merged for both

Agreed, done

notlesh · 2023-01-17T17:54:21Z

precompiles/preimage/src/tests.rs

+	for file in ["Preimage.sol"] {
+		for solidity_fn in solidity::get_selectors(file) {


Whenever I see for loops in unit tests, my first thought is always about what the test would do if the loops stopped running as intended.

For example, if Preimage.sol were renamed, would this test silently stop doing its job (succeeding even though it started running 0 iterations)?

if renamed, would test silently fail

Not sure but I used this test and all tests like this to get the Solidity selectors so I know it's not failing now. This sounds like a reasonable follow up for all precompiles with this test...

notlesh · 2023-01-17T18:01:27Z

precompiles/referenda/src/lib.rs

+	#[precompile::view]
+	fn track_ids(handle: &mut impl PrecompileHandle) -> EvmResult<Vec<u16>> {
+		// Fetch data from runtime
+		handle.record_cost(RuntimeHelper::<Runtime>::db_read_gas_cost())?;


I don't think this comes from storage. Also, it would not account for the scaling per number of tracks.

But maybe those two things cancel each other out, at least for now :)

Indeed, tracks info are static hardcoded data, I think we should not account any gas cost here

We need to account for some, otherwise you could have a malicious contract do a loop over track_ids() to create spam. But it probably doesn't need to be much

Also, it would not account for the scaling per number of tracks. But maybe those two things cancel each other out, at least for now :)

Yes iterating over tracks has some cost. I'll make a follow up ticket. The accurate approach is to benchmark the iteration and charge that weight => gas instead of charging 1 db read.

notlesh · 2023-01-17T18:02:09Z

precompiles/referenda/src/lib.rs

+		UnboundedBytes,
+	)> {
+		// Fetch data from runtime
+		handle.record_cost(RuntimeHelper::<Runtime>::db_read_gas_cost())?;


Same as above

Iterating over tracks has some cost. I'll make a follow up ticket. The accurate approach is to benchmark the iteration and charge that weight => gas instead of charging 1 db read.

girazoki

LGTM, I think there might be some left-overs regarding ts-tests and a bit of refactoring but I think thes can be tackled on a follow-up

precompiles/referenda/src/lib.rs

notlesh

Left some nits, but LGTM otherwise

@nanocryk

* stash precompile changes in other branch and push pallet changes leaking std librar * still leaking std * using sp std phantomdata did not fix * ty @nanocryk for std leak fix * init referenda precompile needs tryfrom u8 for origins * referenda submit precompile fn * signed extrinsics for referenda precompile * preimage precompile * init conviction voting precompile * referenda delegate undelegate unlock * fixes * fix * clean * improve revert reasons for class and index inputs * clean * init accessors and clean * referenda precompile * clean * clean and init referenda precompile interface * fix merge * try mock referenda precompile but did not succeed * save * fix referenda mock * add dot sol files for preimage and voting * add to moonbase precompiles * fix * precompile existence test fix * update solidity addresses as per moonbase config * rename standard vote to vote in conviction voting pallet * start moving origins out of pallet and using additional generic instead * fix finish moving pallet custom origins back into runtime * add opengov precompiles in proxy evm filter * SolidityConvert no longer necessary Co-authored-by: Éloïs <c@elo.tf> Co-authored-by: Stephen Shelton <steve@brewcraft.org> * rm SolidityConvert and add Conviction enum * use trackId as input for submit instead of origin and convert from trackId to origin based on mapping * camelCase error msgs Co-authored-by: Éloïs <c@elo.tf> * Update precompiles/referenda/src/lib.rs Co-authored-by: Éloïs <c@elo.tf> * fix conviction precompile interface * fix referenda precompile interface * test to ensure all tracks have min enactment period less than vote locking period * update tracks based on wiki * revert accidental commit * fix referenda precompile unit tests * update config based on polkadot 6372 gov2 config tweaks * fix * Configure moonriver OpenGov (moonbeam-foundation#2038) * configure opengov for movr * Staking precompile: candidateAutoCompoundingDelegationCount (moonbeam-foundation#2037) * candidateAutoCompoundingDelegationCount * Remove unecessary `.into()` Co-authored-by: Amar Singh <asinghchrony@protonmail.com> * typo Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Amar Singh <asinghchrony@protonmail.com> Co-authored-by: Stephen Shelton <steve@brewcraft.org> * precompiles * address some review todos not done yet * Fixes balance smoke test (moonbeam-foundation#2040) Co-authored-by: Crystalin None <crystalin@Crystalins-MacBook-Pro.local> * trackIds getter and full TrackInfo impl with Curves * patch evm version (moonbeam-foundation#2026) * Removes treasurer in Moonriver * Update eth contract code smoke test (moonbeam-foundation#2043) Change account limit * Adds OpenGov to Moonriver (moonbeam-foundation#2041) * Adds OpenGov to Moonriver * Updates Moonbase * fmt * fixes test * fix impoty Co-authored-by: nanocryk <6422796+nanocryk@users.noreply.github.com> Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Alan Sapede <alan@purestake.com> Co-authored-by: Crystalin None <crystalin@Crystalins-MacBook-Pro.local> Co-authored-by: Crystalin <alan.sapede@gmail.com> Co-authored-by: Tim B <79199034+timbrinded@users.noreply.github.com> * remove TryFrom u8 impl for Origin unused * split convictionVoting vote into voteYes and voteNo so can add voteAbstain in future version without breaking backwards compatibility * fix consistent conversion test * follow suggestions * add doc note to removeOtherVote * fix Co-authored-by: nanocryk <6422796+nanocryk@users.noreply.github.com> Co-authored-by: librelois <c@elo.tf> Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Alan Sapede <alan@purestake.com> Co-authored-by: Crystalin None <crystalin@Crystalins-MacBook-Pro.local> Co-authored-by: Crystalin <alan.sapede@gmail.com> Co-authored-by: Tim B <79199034+timbrinded@users.noreply.github.com>

4meta5 added 2 commits October 20, 2022 14:33

stash precompile changes in other branch and push pallet changes leak…

50562c0

…ing std librar

still leaking std

9395e66

4meta5 added the A3-inprogress Pull request is in progress. No review needed at this stage. label Oct 20, 2022

4meta5 commented Oct 20, 2022

View reviewed changes

pallets/governance-origins/src/lib.rs Outdated Show resolved Hide resolved

4meta5 added 15 commits October 21, 2022 11:13

using sp std phantomdata did not fix

8ed14e7

ty @nanocryk for std leak fix

315116a

init referenda precompile needs tryfrom u8 for origins

8f24362

referenda submit precompile fn

d83be8e

signed extrinsics for referenda precompile

21ffc60

preimage precompile

0eb6dc3

init conviction voting precompile

09e90db

referenda delegate undelegate unlock

138a848

fixes

8e1a709

fix

276d8a4

clean

a940199

improve revert reasons for class and index inputs

a9ab0e3

clean

9067ba0

init accessors and clean

467e845

Merge branch 'master' into amar-gov2-precompiles

c6c8d1b

4meta5 mentioned this pull request Nov 9, 2022

Make public is_passing and ReferendumStatus paritytech/substrate#12667

Merged

4meta5 added 7 commits November 15, 2022 22:09

referenda precompile

4d8f70a

clean

a234655

Merge branch 'master' into amar-gov2-precompiles

def4e70

clean and init referenda precompile interface

e86512c

into master

66e5164

fix merge

c389722

try mock referenda precompile but did not succeed

5bb1673

4meta5 and others added 2 commits January 6, 2023 10:42

save

e7c765b

fix referenda mock

c5e2220

librelois mentioned this pull request Jan 17, 2023

runtime-2100 release #2045

Closed

19 tasks

4meta5 added 4 commits January 17, 2023 10:37

into master

af079a9

remove TryFrom u8 impl for Origin unused

db81510

split convictionVoting vote into voteYes and voteNo so can add voteAb…

d3a8bda

…stain in future version without breaking backwards compatibility

fix consistent conversion test

4c3e81b

4meta5 added A8-mergeoncegreen Pull request is reviewed well. and removed A0-pleasereview Pull request needs code review. labels Jan 17, 2023

Merge branch 'master' into amar-gov2-precompiles

fd381a7

notlesh reviewed Jan 17, 2023

View reviewed changes

precompiles/conviction-voting/ConvictionVoting.sol Outdated Show resolved Hide resolved

notlesh reviewed Jan 17, 2023

View reviewed changes

girazoki approved these changes Jan 17, 2023

View reviewed changes

notlesh reviewed Jan 17, 2023

View reviewed changes

precompiles/referenda/src/lib.rs Show resolved Hide resolved

notlesh reviewed Jan 17, 2023

View reviewed changes

precompiles/referenda/src/lib.rs Outdated Show resolved Hide resolved

notlesh approved these changes Jan 17, 2023

View reviewed changes

nbaztec approved these changes Jan 17, 2023

View reviewed changes

4meta5 and others added 5 commits January 17, 2023 16:57

follow suggestions

be79f7c

add doc note to removeOtherVote

dea9998

into master

052a56e

fix

d4782eb

Merge branch 'master' into amar-gov2-precompiles

3cccdb4

crystalin merged commit f680485 into master Jan 18, 2023

crystalin deleted the amar-gov2-precompiles branch January 18, 2023 08:59

crystalin changed the title ~~Gov2 Precompiles~~ Implement OpenGov Precompiles Jan 20, 2023

4meta5 mentioned this pull request Jan 23, 2023

[MOON-2077] add awarded_pts to precompiles #2056

Merged

notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Feb 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement OpenGov Precompiles #1885

Implement OpenGov Precompiles #1885

4meta5 commented Oct 20, 2022 •

edited

4meta5 commented Jan 4, 2023 •

edited

notlesh Jan 17, 2023

girazoki Jan 17, 2023

crystalin Jan 17, 2023

librelois Jan 17, 2023

4meta5 Jan 17, 2023 •

edited

notlesh Jan 17, 2023

4meta5 Jan 17, 2023

notlesh Jan 17, 2023

librelois Jan 17, 2023

notlesh Jan 17, 2023

4meta5 Jan 17, 2023

notlesh Jan 17, 2023

4meta5 Jan 17, 2023

girazoki left a comment

notlesh left a comment

		for file in ["Preimage.sol"] {
		for solidity_fn in solidity::get_selectors(file) {

Implement OpenGov Precompiles #1885

Implement OpenGov Precompiles #1885

Conversation

4meta5 commented Oct 20, 2022 • edited

Follow ups

4meta5 commented Jan 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4meta5 Jan 17, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

girazoki left a comment

Choose a reason for hiding this comment

notlesh left a comment

Choose a reason for hiding this comment

4meta5 commented Oct 20, 2022 •

edited

4meta5 commented Jan 4, 2023 •

edited

4meta5 Jan 17, 2023 •

edited