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

Change the event we emit when selecting issuers #1706

Merged
merged 3 commits into from
Nov 14, 2019

Conversation

nambrot
Copy link
Contributor

@nambrot nambrot commented Nov 14, 2019

Description

To accomodate #1702, we are changing the event we emit in selectIssuers so that we can keep track of attestations involving validators. This will slightly increase the gas usage of the tx.

Tested

  • Unit tests

Related issues

Backwards compatibility

  • Yes

@@ -98,7 +98,7 @@ contract Attestations is
event AttestationIssuersSelected(
bytes32 indexed identifier,
address indexed account,
uint256 attestationsRequested,
address indexed issuer,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: change event name to the singular, i.e. AttestationIssuerSelected

@asaj asaj assigned nambrot and unassigned asaj Nov 14, 2019
@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@7feadc0). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1706   +/-   ##
=========================================
  Coverage          ?   74.45%           
=========================================
  Files             ?      279           
  Lines             ?     7582           
  Branches          ?      948           
=========================================
  Hits              ?     5645           
  Misses            ?     1823           
  Partials          ?      114
Flag Coverage Δ
#mobile 74.45% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7feadc0...414d3d4. Read the comment docs.

@nambrot nambrot added the automerge Have PR merge automatically when checks pass label Nov 14, 2019
@celo-ci-bot-user celo-ci-bot-user merged commit f29e6d9 into master Nov 14, 2019
@celo-ci-bot-user celo-ci-bot-user deleted the nambrot/attestation-event-for-perf branch November 14, 2019 20:12
aaronmgdr added a commit that referenced this pull request Nov 15, 2019
* master: (81 commits)
  Make styling more consistent in validator quick start and add password prompt to account:unlock (#1731)
  New Version for contractkit (#1727)
  Remove end-to-end attestations test from circle (#1739)
  Added helper function to get the list of current validators (#1713)
  Fix flaky end-to-end transfer, protocol unit tests (#1734)
  Add AccountClaim to Metadata (#1663)
  [Wallet] Set default gradle properties (#1629)
  Update genesis block after adding parent signatures to block header (#1732)
  Removed end-to-end-geth-integration-sync-test job in workflow (#1730)
  Change the event we emit when selecting issuers (#1706)
  [Wallet] Prompt user to restart app when turning off Forno a second time (#1708)
  Check in spanish verification translations (#1726)
  [Wallet] Style fixes on iOS for verification and backup flows (#1718)
  [Wallet] Reset isRedeemingInvite on rehydrate (#1716)
  Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585)
  [Wallet] Add more local currencies (#1698)
  Switch to correct cluster when fauceting (#1687)
  [Wallet] Use the country of the phone number for determining the default local currency (#1684)
  [Wallet] Limit QR code scanner to 1 code per second (#1676)
  Update Dark backgrounds text color (#1677)
  ...

# Conflicts:
#	packages/web/pages/_app.tsx
#	packages/web/src/dev/BuildPage.tsx
#	packages/web/src/dev/Cover.tsx
#	packages/web/src/dev/StackSection.tsx
#	yarn.lock
aaronmgdr added a commit that referenced this pull request Nov 26, 2019
* master: (70 commits)
  Make styling more consistent in validator quick start and add password prompt to account:unlock (#1731)
  New Version for contractkit (#1727)
  Remove end-to-end attestations test from circle (#1739)
  Added helper function to get the list of current validators (#1713)
  Fix flaky end-to-end transfer, protocol unit tests (#1734)
  Add AccountClaim to Metadata (#1663)
  [Wallet] Set default gradle properties (#1629)
  Update genesis block after adding parent signatures to block header (#1732)
  Removed end-to-end-geth-integration-sync-test job in workflow (#1730)
  Change the event we emit when selecting issuers (#1706)
  [Wallet] Prompt user to restart app when turning off Forno a second time (#1708)
  Check in spanish verification translations (#1726)
  [Wallet] Style fixes on iOS for verification and backup flows (#1718)
  [Wallet] Reset isRedeemingInvite on rehydrate (#1716)
  Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585)
  [Wallet] Add more local currencies (#1698)
  Switch to correct cluster when fauceting (#1687)
  [Wallet] Use the country of the phone number for determining the default local currency (#1684)
  [Wallet] Limit QR code scanner to 1 code per second (#1676)
  Update Dark backgrounds text color (#1677)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attestations contract should emit events to track validator performance
3 participants