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

Add OpenGov precompile logs #2071

Merged
merged 23 commits into from
Feb 14, 2023
Merged

Add OpenGov precompile logs #2071

merged 23 commits into from
Feb 14, 2023

Conversation

tgmichel
Copy link
Contributor

@tgmichel tgmichel commented Feb 3, 2023

What does it do?

Adds logs to ConvictionVoting, Preimage and Referenda precompiles.

ConvictionVoting

  • Vote: only when the vote is succesful either yes/no. Frame event: No.
  • VoteRemove: only when the voter is succesfully removed from an ongoing poll. Frame event: No.
  • VoteRemoveOther: only when the target voter is succesfully removed. Frame event: Yes.
  • Delegate: only when the caller succesfully delegates its vote to target. Frame event: Yes.
  • Undelegate: only when the caller succesfully undelegates a previously delegated target. Frame event: Yes.
  • Unlock: always emited when unlock is called, even if there is no unlockeable amount for the account. Frame event: No.

Preimage

  • PreimageNoted: only when the preimage is succesfully registered. Frame event: Yes.
  • PreimageUnnoted: only when the preimage is succesfully de-registered. Frame event: Yes.

Referenda

  • SubmittedAt: only when the referendum is succesfully created. Frame event: Yes.
  • SubmittedAfter: only when the referendum is succesfully created. Frame event: Yes.
  • DecisionDepositPlaced: only when the deposit is succesfully placed. Frame event: Yes.
  • DecisionDepositRefunded: only when the deposit is succesfully refunded. Frame event: Yes.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@4meta5 4meta5 marked this pull request as ready for review February 3, 2023 16:00
@tgmichel tgmichel marked this pull request as draft February 6, 2023 16:51
@tgmichel tgmichel added A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit labels Feb 8, 2023
# Conflicts:
#	precompiles/referenda/src/lib.rs
@tgmichel tgmichel added the not-breaking Does not need to be mentioned in breaking changes label Feb 8, 2023
@tgmichel tgmichel marked this pull request as ready for review February 8, 2023 10:32
@tgmichel tgmichel requested a review from 4meta5 February 8, 2023 10:32
@tgmichel tgmichel changed the title WIP Add OpenGov precompile logs Add OpenGov precompile logs Feb 8, 2023
Copy link
Contributor

@4meta5 4meta5 left a comment

Choose a reason for hiding this comment

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

handle.record_log_costs_manual(X, Y)?; should go at the top of the precompile function body before calls are dispatched

We do not want the calls to dispatch successfully and then the precompile function fails because the caller doesn't have enough gas to pay log costs cc @nanocryk

@tgmichel tgmichel requested a review from 4meta5 February 9, 2023 09:47
# Conflicts:
#	precompiles/preimage/src/lib.rs
#	precompiles/preimage/src/mock.rs
#	precompiles/preimage/src/tests.rs
@tgmichel tgmichel merged commit 4dd9646 into master Feb 14, 2023
@tgmichel tgmichel deleted the tgm-opengov-precompile-logs branch February 14, 2023 16:06
imstar15 pushed a commit to AvaProtocol/moonbeam that referenced this pull request May 16, 2023
* Add vote_yes/no logs

* Comment

* Wip more conviction voting logs

* wip convition voting

* wip preimage

* conviction voting record costs

* Add tests preimage

* cleanup

* Use Hashing assoc type instead Blake explicitly

* Add referenda logs

* Wip referenda log tests

* referenda log tests

* past tense convention for naming events

* updated test

* record log costs first

* fmt

* Merge branch 'master' into tgm-opengov-precompile-logs

# Conflicts:
#	precompiles/preimage/src/lib.rs
#	precompiles/preimage/src/mock.rs
#	precompiles/preimage/src/tests.rs

* Fix version

* Cleanup after rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants