Fix metrics activation values #8302
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yorhodes
pushed a commit
that referenced
this pull request
Aug 11, 2021
### Description To enable metrics we had a small bug that get fixed with this PR. ### Tested Tested in Alfajores
martinvol
added a commit
that referenced
this pull request
Dec 3, 2021
* Build whole phone-number-privacy in docker image (#8268)
* env-tests github action (#8057)
* added env-tests to github actions
* removed environmeent
* initial commit
* support for staging network
* run all tests for staging
Co-authored-by: Kevin Jue <kevjue@Kevins-MacBook-Pro.local>
* Surface inflation rate and factor in network:parameters (#8274)
### Description
Surface inflation rate and factor in network:parameters
### Tested
`celocli network:parameters --node https://forno.celo.org`
<img width="435" alt="Screen Shot 2021-07-09 at 2 49 09 PM" src="https://user-images.githubusercontent.com/3020995/125123949-5ab62e00-e0c5-11eb-9896-3af9d1619ff9.png">
* Allow setting up a different Geth WS port. Private tx-nodes defualts to 8545 (#8161)
* Added wsport as configurable. Setting testnet archive wsport default to 8545
* Added FULL_NODES_WS_PORT values
* Blockscout using new websocket endpoint at 8545
* add section for deploying ETH contract to Celo (#8280)
### Description
add section for deploying ETH contract to Celo
* add protocol differences (#8284)
* add protocol differences
* update to relative links
* SDK 1.2.2-dev --> 1.2.3-dev (#8275)
* changelog added
* versions updated
* keystore added
* chore: resolve to 1.2.3-dev
Co-authored-by: Alex Harley <alexbruceharley@gmail.com>
* add info about Valora accounts (#8293)
* Update devchain docs (#8296)
### Description
This PR updates the the instructions for running a local devchain by referring to the `celo-devchain` package which includes a pre-generated devchain and avoids the need to clone the monorepo and run the migrations.
### Related issues
- Fixes #6758
* Proposal sanity check (#8200)
* some fixes
* simple test is working
* testing this
* still deciding where the checks should be, need cleanup
* cleaned this up
* lint
* review comments
* lint
* New metric exclusions (#8139)
* Added prestop hook to fullnode template. Increased baklava memory
* Included new exclusions for prometheus remotewrite and stackdriver for the more frequent unused metrics
* Refined exclusion
* Update prometheus config
* Added encrypted grafana cloud credentials
* feat: expose indexer metrics (#8116)
Co-authored-by: Enrique Ruiz <enrique@keyko.io>
* Fix metrics activation values (#8302)
### Description
To enable metrics we had a small bug that get fixed with this PR.
### Tested
Tested in Alfajores
* Patch certora governance spec (#8297)
### Description
Fix suggested in [failed CI run](https://app.circleci.com/pipelines/github/celo-org/celo-monorepo/36349/workflows/b03c71a3-61d1-49aa-9d42-fbfbdc3b872d/jobs/476169/parallel-runs/3/steps/3-108) and [slack](https://celo-org.slack.com/archives/CQT5JB2BY/p1626294405043600)
### Tested
CI
* Granda Mento SDK support (#8137)
### Description
Adds support for Granda Mento on contractkit.
### Other changes
-
### Tested
Added tests and modified migrations
### Related issues
- Fixes #7773
### Backwards compatibility
None
### Documentation
-
* Remove timestamp from ODIS (#8298)
* bumps pkg versions
* use new input validation logic
* fix database error metric (drive-by)
* remove timestamp from signer db logic
* sdk changes for using deterministic seed and removing timestamp
* remove timestamp check from combiner
* removes timestamp from combiner getBlindedSig tests
* removes timestamp from signer tests
* updates lockfile
* minor test improvement
* updates lockfile
* fixes error
* bumps contractkit versions in signer and combiner
* ODIS Quota take cEUR balance into account (#8205)
* Added cEURO to quotes for account validation
* Updated ContractKit in phone-number-privacy package
* Updated Identity dependency in Combiner
* Updated code with PR comments
* Added missing yarn.lock
* Updated logged message in Quote service
* Update yarn.lock
* Update signer dependency
* Update dependencies in combiner
Co-authored-by: Alec Schaefer <alec@cLabs.co>
* Invite Rewards Deployment (#7835)
### Description
Deployment automation for creation, updating, deleting of Celo Rewards Service.
### Other changes
Remove the private key (mnemonic) method for relayer deployment. This was causing deployment issues and we weren't using it.
### Tested
Verified in Alfajores.
### Related issues
- Fixes #7835
### Backwards compatibility
Addition of a new micro-service. Does not interfere with existing Komenci services.
### Documentation
N/A
* various fixes from ODIS v1.1.8 deployment (#8309)
various minor fixes and cleanup noticed during ODIS v1.1.8 deployment and e2e testing
* Add missed variable to enable metrics (#8310)
* Add missed variable to enable metrics
* Use the variable
* Do not include ws port in service if is 8545 (#8319)
### Description
Fix testnet package if wsPort is 8545 (services cannot have same port defined multiple times)
* Prometheus remote_write refactor and included rc1 grafanaCloud (#8320)
### Description
Small refactor to prometheus remote_write due to old approach (using helm `toYaml` funcion) added single quotes to some fields that should not be included in the template.
Also included the encoded variables for grafana cloud in rc1 environment.
### Tested
Deployed to rc1 mainnet cluster.
* Update release process docs to reflect simpler tagging scheme (#8267)
### Description
Fixes contract release process scripts and docs to use simplified tagging scheme
### Tested
CI
### Backwards compatibility
Yes
### Documentation
Smart contract release process docs
* Don't use mycelo for e2e tests by default. (#8291)
### Description
Following up on #8086, we realized that using mycelo for the tests by default introduces friction for developers making changes to the core contracts, since it requires them to also update the migrations which mycelo uses, which are in the celo-blockchain repo and in Go rather than javascript. So as a compromise, this PR modifies the tests back to not use mycelo by default. Instead, they can be switched by devs to use mycelo temporarily (for local runs) as desired, or forced to use mycelo by setting an env variable:
```
export E2E_TESTS_FORCE_USE_MYCELO=true
```
We expect that for the celo-blockchain CI we will set this environment variable, but we won't for the monorepo CI.
### Tested
Verified that the tests now don't use mycelo, but do if you specify the env variable to be true.
* Implement contract upgrades compatibility in ContractKit (#8308)
### Description
- Adds version logic to ContractKit base wrapper to support generic version gatekeeping
### Other changes
- Modularize and simplify protocol build scripts to only generate artifacts when necessary
- Modify contractkit build:gen script to support targeting contract releases
### Tested
- Tests added for base wrapper version compatibility
### Related issues
- Fixes #8018
### Backwards compatibility
Yes
### Documentation
None
* Twilio Verify API (#8189)
### Description
The Twilio Verify Service is a managed service for sending N-digit codes globally. Because there's no risk of sending marketing material, Twilio can automatically manage regulations for each recipient geography on behalf of the sender. The service also manages localization for select markets. We chose Twilio as the first SMS provider to measure the impact of switching to a Verify Service since it 1) is heavily used by many operators 2) supports sending custom codes which minimizes Valora-side changes.
Changes to note:
- Changes SMS text content
- Android
- from `<#> Celo security code is: 12345678 bU9E4ctGtIW`
- to `<#>Your Celo verification code is: 12345678 bU9E4ctGtIW`
- iOS
- from `Celo security code is: 12345678`
- to `Your Celo verification code is: 12345678`
- Expands localized markets from 3 to 36
- Doesn't support a call-back URL which will affect some metrics on our dashboard
- Supporting verification for landlines (using voice)
Pre-requisite steps for Attestation Service operators:
- Create a new Verify Service in the portal
- Create a support ticket to enable custom security codes
### Other changes
We now include `language` and `appSignature` in the `AttestationModel` object. The Verify API uses these directly to adjust the messaging. Added a new migration file for adding these columns to the Attestations table.
Added `twilioVerifySidProvided` to the /status endpoint and tracking via metadata-crawler.
### Tested
Tested manually in Alfajores using Valora Alfajores app:
- Android device receives app hash and can auto-read message
- Can re-request SMS from same services
- Successfully retrieves signature from AS using security code
### Related issues
- Fixes #8145
### Backwards compatibility
`AttestationModel` objects that are previously stored in the DB do not have the new properties (`language` and `appSignature). When reading the `AttestationModel`, these new properties are be added for compatibility.
### Documentation
Updated Twilio guide
* Pass APIs enabled from testnet to common packages (#8350)
### Description
Setup RPC_APIs configuration parameter in testnet templates.
* Add blockscout mainnet staging env (#8348)
Blockscout staging env config
* Remove unneccesary getVersionNumber() function declaration in contract libraries (#8349)
### Description
Current Behavior: Multiple contract libraries have versioning bytecode that is overwritten by inheriting contracts or simply unused
New Behavior: Removed unnecessary versioning in library contracts. Only difference in functionality would be if one were attempt to query the version of a library with getVersionNumber(), although this is seldom done in practice
### Testing
Changes do not need to be tested specifically, since getVersion was not called on libraries.
### Related issues
- Consistently unversion contract libraries #7435
### Backwards compatibility
Changes are backwards compatible since the functionality being removed was unnecessary and not used.
### Documentation
N/a
* Move GrandaMento from liquidity dir to stability (#8300)
### Description
CI doesn't automatically look at new directories, so just moving GrandaMento to the stability directory.
### Other changes
n/a
### Tested
Building & testing
### Related issues
n/a
### Backwards compatibility
Yes
### Documentation
n/a
* feat: expose blockscout prometheus metrics (#8358)
* Remove packages/contractkit (#8258)
### Description
See #5999.
Now that everything is sorted we can safely remove this package (ContractKit lives under packages/sdk).
* Clarify requirement to use Node.js v12.x (#8359)
### Description
This PR aims to make it clearer from the setup instructions that Node.js v12.x must be
used when working with the monorepo.
* Added D'CENT Wallet (#8367)
https://medium.com/celoorg/celo-and-dcent-bringing-greater-access-to-defi-dapps-in-korea-82bb80577b76
* GrandaMento comment fixes & clearer revert msgs - N01 and N02 (#8344)
### Description
Includes fixes for:
* #8331 (Unclear revert msg for `cancelExchangeProposal`)
* #8327 (Unfinished & ambiguous comment)
### Other changes
n/a
### Tested
Unit tests
### Related issues
- Fixes #8331
- Fixes #8327
### Backwards compatibility
Backward compatible
### Documentation
N/a
* add tests for nixSecondsTimestampToDateString (moment removal prework) (#8362)
* WIP add test for the function without any tz mocking
* add timezone-mock to enable determinist date testing
* Loadtest celotool client: Included threading and multiple light-clients in the same pod (#7373)
### Description
Added the threading functionality to the load-test client included in celotool. Included also celotool funcions to help with the fauceting process. The PR includes a complete refactor of the eth load-test functionality, with the most notable changes:
- Added new celotool command `faucet-load-test` to send `CELO` and `cUSD` to any subset of load-test accounts.
- Added new celotool command `prepare-load-test` meant to be run as a init-container for each pod in the load test chart. It setups ethereum accounts and configuration files that will be used by the load-test clients.
- Added multi-threading. Each deployment can be configured with `replicas` (number of pods, each one using a different ethereum client to connect to the client), and `threads` that are different load-test clients inside the same process and sharing the same ethereum client (but using different accounts)
### Tested
Tested extensively in testnets and during performance experiments.
### Related issues
- Fixes celo-org/celo-blockchain#1354
### Backwards compatibility
The changes are isolated to the load-test functionality.
* Custom values.yaml for each context in celo-fullnode chart (#8191)
### Description
Different deployments with different needs are using the celo-fullnode chart currently. Some of the configuration parameters for this have to be configured chart-wide or env wide (as most variable are not contexted). Following the pattern used in blockscout, this PR adds the capability of adding a custom values.yaml file for each context so we can customize each deployment without adding a lot of env variables for each context. The custom value file has to be `packages/helm-charts/celo-fullnode/{env}-{context}-values.yaml`.
Added two custom values for rc1-gcp-private-txnodes and baklava-gcp-forno-europe-west1 contexts.
### Other changes
No
### Tested
Tested in baklava forno context.
### Related issues
- Fixes #[issue number here]
### Backwards compatibility
No problem
### Documentation
_The set of community facing docs that have been added/modified because of this change_
* Minor tweaks to "Stable Cryptocurrencies" (#8333)
### Description
_A few sentences describing the overall effects and goals of the pull request's commits.
What is the current behavior, and what is the updated/expected behavior with this PR?_
### Other changes
_Describe any minor or "drive-by" changes here._
### Tested
_An explanation of how the changes were tested or an explanation as to why they don't need to be._
### Related issues
- Fixes #[issue number here]
### Backwards compatibility
_Brief explanation of why these changes are/are not backwards compatible._
### Documentation
_The set of community facing docs that have been added/modified because of this change_
* GrandaMento - Add hardcoded max for veto period - L07 (#8368)
### Description
Addresses #8330, which calls out that hardcoded min/max values for the veto period would be useful.
* After chatting with Albert, we think there is value in a hardcoded maximum value (set to 4 weeks), but want to preserve the flexibility of being able to set the veto period as low as 0. If down the line the community were to feel that the multisig is not necessary and the `approver` should instead be set to Governance directly, setting the veto period to 0 is desirable. So this PR just adds a `require` enforcing `vetoPeriodSeconds` as <= 4 weeks.
* Apparently there were no existing tests for `setVetoPeriodSeconds`, so they were added
### Other changes
n/a
### Tested
Unit tests
### Related issues
- Fixes #8330
### Backwards compatibility
Backward compatible
### Documentation
n/a
* GrandaMento - Lock in veto period in proposal - L01 (#8369)
### Description
* Addresses #8322 by storing `vetoPeriodSeconds` in each proposal so an active proposal is not affected by a change to the contract's veto period
### Other changes
n/a
### Tested
Unit tests
### Related issues
- Fixes #8322
### Backwards compatibility
Backward compatible
### Documentation
n/a
* removes moment.js by replacing with Intl.DateTimeFormat (#8451)
remove timezone-mock as its too limited in scope to properly mock (doesnt support Intl or Date.toLocalDateTimeString())
fix some typos
* Updated Grafana ingress to k1.18+ (#8462)
### Description
Updated grafana ingress resource to 1.16+ k8s specification.
* Obasilakis/expose blockscout prometheus metrics (#8448)
* feat: expose blockscout prometheus metrics
* fix: remove spaces that break deployment
Co-authored-by: Donald Hutchison <git@toastymofo.net>
* Revert fixes for audit 6/release 5 on master (#8471)
- Revert "GrandaMento - Lock in veto period in proposal - L01 (#8369)"
- Revert "GrandaMento comment fixes & clearer revert msgs - N01 and N02 (#8344)"
- Revert "GrandaMento - Add hardcoded max for veto period - L07 (#8368)"
* Updated tar dependency to 4.4.15 (#8466)
* Update metadata-crawler image in blockscout helm charts (#8467)
### Description
- changes metadata-crawler image to `77a392216d4927e85ce4b683508fc0539aa92a34` (includes `Verify API changes`)
- Note: also changes this value in `helm-charts/blockscout/values.yaml` (hasn't been changed in a while, but should be the same image tag referenced I believe)
* Added test to odis common code (#8351)
### Description
I added a few tests to _common_ package of _phone-number-privacy_.
Coverage :
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------------------|---------|----------|---------|---------|---------------------
All files | 91.67 | 90.63 | 100 | 91.67 |
interfaces | 100 | 100 | 100 | 100 |
error-utils.ts | 100 | 100 | 100 | 100 |
index.ts | 100 | 100 | 100 | 100 |
utils | 88.75 | 89.29 | 100 | 88.75 |
authentication.ts | 83.02 | 75 | 100 | 83.02 | 52-55,66-70,128-130
constants.ts | 100 | 100 | 100 | 100 |
input-validation.ts | 100 | 100 | 100 | 100 |
* Set blockscout k8s deployments to include erlang node name (#8476)
* Set ws port to the same as http for blockscout.
* Add default values for sourcify.
* Apply node names to blockscout startup commands.
* Use default route instead of localhost for node name.
* Set full node name in deployments.
* Rename nodeName to erlangNodeName.
* Remove duplicate sourcify.
* Update wallets.md to show Valora as maintainer of Valora (#8483)
### Description
docs updated to show that Valora maintains Valora (requested by Denisse [here](https://valora-app.slack.com/archives/CL7BVQPHB/p1628185335065300) )
### Other changes
n/a
### Tested
previewed the rendered page
### Related issues
n/a
### Backwards compatibility
n/a
### Documentation
This page is updated: https://docs.celo.org/getting-started/wallets
* add accessors for target symbols and weights of reserve target allocations. (#8481)
* Add --txpool.nolocals to forno full nodes (#8499)
### Description
A small change made along with @piersy when forno was having some issues. `--txpool.nolocals` is recommended when exposing nodes publicly: https://github.com/ethereum/go-ethereum/issues/18242#issuecomment-444073885.
We noticed it took a really long time for the command to find the most recent block timestamp, which was due to the `transactions.rlp` file being loaded. Adding `--txpool.nolocals` causes that to not be loaded or written to.
I added the `--txpool.nolocals` only for GCP-based full nodes, which I believe are only used for forno (@jcortejoso correct me if I'm wrong or if this will mess other infrastructure up that I'm not familiar with).
### Other changes
n/a
### Tested
Deployed forno full nodes
### Related issues
n/a
### Backwards compatibility
backward compatible
### Documentation
n/a
* Re-instate prior timeout to retrieve latest block (#8505)
The timeout was increased to 600 to overcome the time taken to process
the txpool local txs, but due to the addition of the -txpool.nolocals
flag that extended timeout is no longer required.
* fix CI (#8516)
* Upgrade eksportisto (#8518)
### Description
Upgrade eksportisto to the latest version which includes this PR:
- https://github.com/celo-org/eksportisto/pull/58
* add metamask and wallet connect (#8523)
* Enabled metrics, changed the pool size (#8524)
### Description
Enabled blockscout metrics by default.
### Other changes
Increased the connection pool size.
### Tested
Tested on all envs.
### Backwards compatibility
Yes.
* fixed typo in word "governance" (#8526)
* Separate testnet from production decryption permissions (#8529)
* Separate testnet from production decryption permissions
* Remove debug msg
* Granda Mento env test, small env test refactor and fixes (#8335)
### Description
This includes:
* An env test for GrandaMento that tests selling and buying CELO for: (1) creating & immediately cancelling an exchange proposal and (2) creating and executing an exchange proposal.
* Small refactor of env-tests to use contractkit's `StableToken` & related functions for stable token specific things
* Added CELO fauceting in env-tests
* Some small changes to the GrandaMento contractkit wrapper
* Added stable token exchange limits to the GrandaMento migrations
### Other changes
* Small fix to testnet helm chart
### Tested
Ran against a testnet I created
### Related issues
- Fixes #8065
### Backwards compatibility
They are backward compatible
### Documentation
n/a
* Update signer image to latest (#8533)
### Description
This version is already deployed. Just updating our env files to reflect it.
* Merge release 5 to master (#8475)
### Description
Merge audit fixes triaged in https://github.com/celo-org/celo-monorepo/projects/2
### Tested
CI
### Related issues
- Fixes #8437
### Backwards compatibility
Yes
### Documentation
_The set of community facing docs that have been added/modified because of this change_
* ODIS matchmaking replay handling (#8357)
### Description
Accounts can now query matchmaking unlimited times as long as they use the same phone number and provide a DEK signature of that phone number in the request
### Other changes
Adds DB migration instructions to Combiner and Signer READMEs
Adds functionality for e2e tests to bypass verification check in combiner on alfajores when using specific fake phone numbers
Adds `yarn update-dependency-graph` command to root package.json
Adds documentation on ODIS replay handling for both signing and matchmaking
Updates auto generated ODIS docs
Factors DEK signature verification into its own function in common package
Adds custom `seed` parameter to blinding client in `@celo/identity`
Adds custom `blindingFactor` parameter to `getPhoneNumberIdentifier` in `@celo/identity`
Adds `dekSigner` parameter to `getContactMatches` in `@celo/identity`
### Tested
Unit and e2e tests
### Related issues
- Fixes https://app.zenhub.com/workspaces/cap-sprint-board-60059934ed9c6f000e5594f6/issues/celo-org/celo-monorepo/8312
### Backwards compatibility
Yes
### Documentation
Updated in PR
* doc: Update foundation voting policy (#8539)
* doc: Update foundation voting policy
* Update celo-foundation-voting-policy.md
* Flake tracker error handling (#8541)
### Description
Adds error handling to flake tracker so that CI doesn't fail if there's an issue
### Other changes
None
### Tested
In CI
### Related issues
- Fixes CI blocker
### Backwards compatibility
Yes
### Documentation
None
* Fix governance encoding and decoding for unregistered contracts (#8537)
### Description
- Handles unregistered contracts in governance tooling
- Adds `--afterExecuting{ID|Proposal}` to `governance build:proposal` for batching support
### Other changes
- Unify unregistered contracts handling in sdk
### Tested
against staging
`./bin/run governance:show --node http://localhost:8545 --proposalID 19`
```yaml
proposal:
0:
contract: Registry
function: setAddressFor
args:
0: GrandaMento
1: 0xF9F8bfFB41883A0a224400C2CA3Aa629ae8156DC
params:
identifier: GrandaMento
addr: 0xF9F8bfFB41883A0a224400C2CA3Aa629ae8156DC
value: 0
1:
contract: GrandaMentoProxy
function: _setAndInitializeImplementation
args:
0: 0xFA5696a00b612f23386fE0a58AdccB78c76012Fc
1: 0xd13f90b4000000000000000000000000000000000000000000000000000000000000ce100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003f870857a3e0e380000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000127500
params:
implementation: 0xFA5696a00b612f23386fE0a58AdccB78c76012Fc
callbackData: 0xd13f90b4000000000000000000000000000000000000000000000000000000000000ce100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003f870857a3e0e380000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000127500
initialize@d13f90b4:
_registry: 0x000000000000000000000000000000000000ce10
_approver: 0x0000000000000000000000000000000000000000
_maxApprovalExchangeRateChange: 300000000000000000000000
_spread: 0
_vetoPeriodSeconds: 1209600
value: 0
```
`./bin/run governance:build-proposal --node http://localhost:8545 --output test.json --afterExecutingID 19`
```
After executing provided proposal, account for registry remappings: {
"GrandaMento": "0xF9F8bfFB41883A0a224400C2CA3Aa629ae8156DC"
}
Transaction #1:
? Celo Contract: GrandaMento
? GrandaMento Function: setStableTokenExchangeLimits
? stableTokenRegistryId: StableTokenEUR
? minExchangeAmount: 100
? maxExchangeAmount: 10000
Transaction #2:
? Celo Contract: ✔ done
Outputting proposal to test.json
Simulating proposal execution
```
* Bump wallet connect version (#8215)
### Description
- Bump versions of WalletConnect libraries
- CAIP10/50/short-name address parsing
### Other changes
N/A
### Tested
Unit and E2E tests
### Related issues
N/A
### Backwards compatibility
N/A
### Documentation
N/A
* GitBook: [master] 1,404 pages modified
* GitBook: [master] 3 pages modified
* GitBook: [master] one page modified
* GitBook: [master] 2 pages modified
* Revert "GitBook: [master] 1,404 pages modified" (#8575)
This reverts commit 1f3780da16702b8af07f69aad12b0c88e53a1374.
* testnet loadtest small fixes (#8579)
### Description
Small fixes or the loadtest in networks deployed with mycelo
* Impemented a smoother scale down policy for blockscout api hpa (#8581)
### Description
Defines scale up and scale down behavior for blockscout API HPA.
Scales up if the average CPU usage is higher than the defined threshold (currently 70%), doubling the number of pods or adding 2 pods every 30 seconds whichever affects the higher number of pods.
Scales down after 10 minutes removing 20% of pods or 1 pod every minute whichever affects the smaller number of pods.
### Tested
Tested on staging.
### Documentation
https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/#support-for-configurable-scaling-behavior
* Granda Mento support in CLI (#8534)
### Description
As described in #7774
### Other changes
- Generalized functions on contractkit to be used for the cli testing as well
### Tested
Added unit tests.
### Related issues
- Fixes #7774
### Backwards compatibility
-
### Documentation
- Added, auto generated.
* Upgrade oracle version.
Also point all three networks (baklava, alfajores and mainnet) to the
same container registry.
* Tobikuhlmann/adjustment factor testing (#8571)
* add testing documentation to protocol readme
* refactor epoch time travel
* extensive adjustment factor testing
* add tests and fix total supply & reserve initialisation numbers
* remove test-utils.ts changes
* update epoch rewards init parameters
* refactor test init
* Update packages/protocol/test/governance/network/epochrewards.ts
Co-authored-by: Roman Croessmann <42186452+rcroessmann@users.noreply.github.com>
* fix typo
* add one year tests
* reframe 'year' to '365 epochs'
* work in lint feedback
Co-authored-by: Roman Croessmann <42186452+rcroessmann@users.noreply.github.com>
Co-authored-by: Nelson Taveras <nelson@clabs.co>
* Incorporate js keystore (+ fallback nodes) into attestation service (#8257)
* Incorporate js keystore into attestation service with backwards compatibility
* Include keystores in package.json
* Fix linting error
* PR fixes
* Update dependency graph
* Copy keystores package into AS dockerfile
* Update attestation service deployment docs (#8264)
* Update attestation service deployment docs
* Wording suggestion (PR review)
Co-authored-by: Cody Born <codyborn@outlook.com>
* Nit wording
* Fix instructions around starting AS docker container
* Correct old typo
* Add forno to docs
* Clarify submission vs reading from chain
* Tiny rephrase
* Docs fixes for single celo provider method
* Fix link and rephrase
Co-authored-by: Cody Born <codyborn@outlook.com>
* Implement fallback on error only
* wip
* Implement just-in-time prioritization
* Simplify selection logic
* Use isYes for env var processing
* Add timeout handling but wip
* Clean up kit init logic
* wip: implementation working
* Fixed bug and added periodic edge case check
* Clean up some constants from init
* Remove console logs and add one log statement
* Update env variables
* Fix typo
* Fix lint checks
* Use timeout instead of withTimeout
* Update package version pre-testing
* Update docs
* Update helm charts and celotool for deployment with backup provider
* wip includes deployment image hash
* Fix linting
* Update packages/attestation-service/src/db.ts
Co-authored-by: Cody Born <codyborn@outlook.com>
* Add docstrings to initializeKits and execWithFallback
* Update image tag to latest 1.4.0-dev build
Co-authored-by: Cody Born <codyborn@outlook.com>
* Documentation update (#8554)
### Description
Added two new pages with instructions to connect a Ledger device to Celo Web Wallet & Celo Terminal
* Combiner min instance count (#8593)
### Description
We saw some errors in the Combiner logs that indicated that their weren't enough instances allocated; despite very low traffic. Setting a min instance count to prevent minor spikes in traffic from not being served.
> The request was aborted because there was no available instance
### Tested
Testing in staging before merging.
### Backwards compatibility
Yes.
### Documentation
N/A
* Add MINIMUM_PRICE_SOURCES to Oracle's helm chart.
* Set the Oracles' minPriceSourceCount to 2.
* Using Metamask with Celo (#8592)
Adding back doc updates lost in revert. Using MetaMask with Celo updates to clarify warnings and tutorial language.
* Update to new testnet encryption key (#8566)
### Description
We're deprecating the old encryption key (`github-key`) to replace with a new one that is environment specific (prod vs non-prod).
### Tested
Encrypted/Decrypted using the new key
### Related issues
- Fixes #8542
### Backwards compatibility
Updating .enc files so the old key is no longer necessary
* Add annotation with information about deployment for grafana dashboards (#8587)
* Add annotation with information about deployment for grafana dashboards
* Config as environment variables
* Fix end lines
* Fix comments
* Truncate error in Attestation Service (#8536)
### Description
Error message in DB is max of 255 chars and throws the following exception when the error is longer than this:
`value too long for type character varying(255)`
This prevents retries during failure since this breaks out of the retry loop.
### Tested
Tested with a single attestation service for a week. Verified no errors of this type came through.
### Related issues
- Fixes #[issue number here]
### Backwards compatibility
Yes.
### Documentation
N/A
* azure front door fallback (#8532)
* update signer dependency
* Added fallback for signers.
* Added test and options to Circuit Breaker
* Removed console.log
* Added circuit breaker for every signer
* Corrected import in Combiner
* CLI governance:propose support for dependencies on existing proposals (#8550)
### Description
This was needed to create the GrandaMento "activation" governance proposal on staging while CR5 (the proposal that would set GrandaMento in the registry & initialize the contract) was not executed yet.
* Brings out the `--afterExecutingProposal` and `--afterExecutingID` logic from `governance:build-proposal` and reuses it in `governance:propose`
* Allows registry additions to be passed into `proposalToJSON`
### Other changes
n/a
### Tested
Was able to propose the GrandaMento activation governance proposal with the use of `--afterExecutingID`
### Related issues
n/a
### Backwards compatibility
Fully backward compatible
### Documentation
Doc changes autogenerated
* Fix `kit.getNetworkConfig()` failure mode when some contracts are not yet registered (#8569)
### Description
Fixes fetching network parameters when some sdk contracts are unregistered as followup to https://github.com/celo-org/celo-monorepo/pull/8537
### Tested
<img width="555" alt="Screen Shot 2021-08-30 at 8 00 45 PM" src="https://user-images.githubusercontent.com/3020995/131434692-d9d9f613-a7ac-4cad-97cd-65f6a7940c57.png">
* Simplify attestation service deployment in prep for release 1.4.0 (#8607)
### Description
Simplifies container deployment and should remove an issue we occasionally saw when the genesis block was incorrect in GCS. Runs the geth node in the updated, post-donut way.
### Other changes
- updates the release process alfajores deployment slightly (+ docs to reflect that), to make it more consistent with baklava + mainnet release tagging. With these slight changes, we would try to ensure that `attestation-service-alfajores` reflects the candidate release version that is currently deployed on alfajores, and can test + make changes to this before tagging the official release on GitHub. (`attestation-service-baklava` is already supposed to, so this would also make the two tags consistent + the one that we deploy)
(- did not change this but maybe we should revisit whether we want to remove the baklava release section if that is no longer part of the process?)
- bumped AS version to `1.4.0` (from `1.4.0-dev`...I can separate this into its own PR if this clutters this PR too much, but that feels like overkill since this the AS as is should be the 1.4.0 release candidate 😅)
### Tested
- tested deployment, checked container logs to make sure startup worked as expected and that the geth ultralight client syncs properly (there is one "db already initialized" error in the logs @codyborn but does not block proper functioning afterwards; this is the case with/without the new deployment changes)
### Related issues
- Fixes #8460
- Related to #8244
* Add author in grafana annotation (#8616)
### Description
Add author in grafana release annotation
* Added tests to Attestation Service (#8621)
* update signer dependency
* Added fallback for signers.
* Added test and options to Circuit Breaker
* Removed console.log
* Added circuit breaker for every signer
* Corrected import in Combiner
* Added tests
* Added working package.json for new tests
* remove the 'd' package
* Change yarn lock
* Adjusted blockscout request parameters (#8627)
### Description
Based on observation from monitoring:
- decreases CPU requests for db proxy container for indexer, web and api.
- increases CPU requests for api and web, and triples the connection pool size per pod, at the same time halving the max number of replicas in each HPA.
* Create Certora spec for Reserve spender (#8572)
### Description
This PR adds a Certora spec for the Reserve contract, so far with one rule: checking that the Reserve's funds cannot be moved by an address that's not an approved spender.
### Other changes
* Added this as a CI job
* Set all spec-related scripts as executable
### Tested
Spec passes when running locally
### Related issues
- Fixes #8513
* Emilhotkowski/fallback without circuit (#8632)
* update signer dependency
* Removed circuit breaker
* Added logging
* Add `grandamento:execute`, `grandamento:get-buy-amount`, small fixes, and some Granda Mento wrapper fns (#8612)
### Description
* Adds `grandamento:execute` to the cli, which executes an approved exchange proposal that has passed the veto period since the time of approval
* Adds `grandamento:get-buy-amount` to the cli, which calculates the amount of the buy token a proposal would receive according to the current oracle price & the spread
* Adds some wrapper functions to the contractkit GrandaMento wrapper
* Removes some unnecessary `await`s that my editor found
* Ensures all the `grandamento` CLI commands run their checks
### Other changes
n/a
### Tested
Unit tests, also ran the commands
### Related issues
n/a
### Backwards compatibility
Fully backward compatible
### Documentation
Autogenerated CLI docs
* Remove validator group confirmation for revoking votes (#8637)
* Forno rate limit using Kong (#8100)
### Description
This PR adds a new celotool package to install [Kong](https://github.com/Kong/kong) and [Konga](https://github.com/pantsel/konga).
This two services are used to implement a rate limit based on client's IP in Forno.
It includes the new celotool package (named `kong`) that installs kong and konga, the helm charts for deploying and configuring kong and konga (kong's configuration is deployed/updated when upgrading the kong package), and terraform forno updates to include the new setup.
### Other changes
No.
### Tested
Tested in baklava forno and already deployed in mainnet forno (not active yet)
### Related issues
- Closes celo-org/celo-blockchain#1570
### Backwards compatibility
I do not expect problems.
### Documentation
Internal usage.
* Do not scale fullnodes when --helmdryrun (#8563)
### Description
Skip scaling up or down the fullnodes if `--helmdryrun`.
### Other changes
Updated forno values.
* Critesjosh/deprecate docs package (#8700)
* delete docs, update README
* fix typos
* update yarn.lock
* remove docs package
* adds checkSequentialDomain + tests, SequentialDelayDomain (#8701)
### Description
Adds checkSequentialDomain function for parsing rate limits for the new SequentialDelayDomain
### Other changes
None
### Tested
Unit Tests
### Related issues
- Fixes https://app.zenhub.com/workspaces/identity-backlog-board-60e61301c3dc55000e540b61/issues/celo-org/celo-monorepo/8640
### Backwards compatibility
Yes
### Documentation
No
* Release/sdk/1.2.5 (#8667)
### Description
Release 1.2.5 of the SDK and 1.3.0 of the CLI.
### Other changes
Stole a [commit](https://github.com/celo-org/celo-monorepo/pull/8301/commits/be5220f6794e829e81934c2b9814a87c44d9b52a) from the last [1.2.4 release](https://github.com/celo-org/celo-monorepo/pull/8301) because it never got merged.
### Tested
N/A.
### Related issues
- Fixes https://github.com/celo-org/celo-monorepo/issues/8553
### Backwards compatibility
N/A
### Documentation
N/A
* Emilhotkowski/added endpoints for rate limiting (#8654)
### Description
Added endpoints and domain model for RateLimiting.
### Backwards Compatible
Yes
### Tested
No tests yet.
* [Celostats] Environment point to master (#8730)
* Bugfix/blockscout compilation (#8727)
### Description
CI images are being built with cached dependencies yet there are some source files with a more recent mtime than the cached version. This results in a recompilation on start up which takes longer than the k8s readiness check will permit. Until we migrate the deployment to use mix releases (fully contained binary releases) this PR is a work around to prevent recompilation on startup.
### Tested
* ran a full filesystem diff on containers built from cache and compiled directly
* filesystem contents identical
* deployed to staging with these helm charts
* startup ok
### Related issues
- celo-org/data-services#53
- celo-org/data-services#57
* Redirect coin balances (#8753)
### Description
This PR will fix the redirect for coin balances
### Other changes
_Describe any minor or "drive-by" changes here._
### Tested
In alfajores
### Related issues
- Fixes https://github.com/celo-org/data-services/issues/55
### Backwards compatibility
Yes
* Blockscout nginx redirects (#8754)
### Description
This adds redirects to the default Blockscout ingress as well.
### Other changes
-
### Tested
Tested on all envs.
### Related issues
- A follow-up for https://github.com/celo-org/data-services/issues/55
### Backwards compatibility
Yes.
* Fix WalletConnect V2 (#8691)
* fix: wc beta-18 and updating chain ids
* fix: update signing chain id
* fix: Await connect
Co-authored-by: Daniel Kesler <danielckesler@gmail.com>
* GKE Prometheus metrics: set up GKE workload metrics (#8760)
* Add new PodMonitor helm chart
See https://cloud.google.com/stackdriver/docs/solutions/gke/managing-metrics#configure-workload-metrics.
Create a generic config with broad exclusions and inclusions.
* Add more endpoints and dynamically create a mapping of endpoints and labels
* Correct dashes and port values
* Correct typo
* Put dashes on the left
* Change targetPort back to port, as the CRD does not support targetPort
This was an attempt to easily use unnamed ports.
* Correct the PodMonitor instance by adding a selector
Turns out the selector cannot be empty with this CRD.
Add a dummy selector to catch as many prod service pods as possible.
* Blockscout web and api both use /metrics/web for prometheus metrics
* Correct refactory typo for the port value
* Expand the PodMonitor from the Blockscout single component proof of concept
Create 1 PodMonitor config per label + endpoint config.
Use Helm magic to reduce repetition at the cost of making it a bit harder to parse.
Long term it would make sense to align the Prometheus ports to a single config.
Tested on Alfajores.
* Use Release.Name for the PodMonitor metadata
* Add a README and NOTES
* Add kube state metrics
* Add TODOs for full coverage
A few pods don't have a named port and need one to be monitored.
kube-state-metrics needs migrating to the up to date charts.
* Refactor the podmonitors to 1 per service
Make use of the fact that each podmonitor can have multiple podMetricsEndpoints
and use even more Helm templating magic to have a 1:1 relationship between
a service and a monitor.
Also add the endpoint used for the testnet validators.
* Refactor the podmonitors to 1 per service
Make use of the fact that each podmonitor can have multiple podMetricsEndpoints
and use even more Helm templating magic to have a 1:1 relationship between
a service and a monitor.
Also add the endpoint used for the testnet validators.
* Only create PodMonitors in the clusters where a service is present
* Add celotool support for the new GKE Workload Metrics
Add it to the prometheus cmd.
* Refactor GKE workload metrics helm charts to support clusters and namespace pairs
Some namespaces are reused across multiple clusters to form one "celo environement", eg baklava and rc1.
Augements the config to explicitly list all the cluster and namespace pairs where the podmonitors should search for stuff to monitor.
* Fix yarn lint errors
* Change the default cluster name in the Helm values to a placeholder
Prevents accidentally using the chart without specifying a cluster.
* Use the cluster context to enable the GCP Workload Metrics setting
* Cosmetic changes
- Use install instead of upgrade for the initial deployment in all prometheus commands
- Shorten the code
- Clarify log message
* Bump cross-fetch dependency of contractkit (#8772)
### Description
Bumps vulnerable ContractKit dependency (see issue)
### Tested
CI
### Related issues
- Fixes #8521
* Update Contractkit in ODIS (#8776)
* Separate testnet from production decryption permissions
* Update firebase + node version
* Update contractkit
* Remove future changes
* Add `celocli releasegold:admin-revoke` command (#8749)
### Description
Adds a command which can be repeatedly used against individual ReleaseGold contract instances to ensure the releaseOwner makes progress in revoking and finalizing a grant.
### Other changes
- Adds utilities to ReleaseGold, Election, Governance contractkit wrappers
### Tested
CLI tests against devchain
### Related issues
- Fixes #8696
### Backwards compatibility
_Brief explanation of why these changes are/are not backwards compatible._
### Documentation
Command is hidden
* Infrastructure changes for Eksportisto 2.0 (#8580)
* Wip
* Setup an eksportisto-2.0 chart and make changes to deployment script
* Finalize node allocation logic
* Deploy new version with differnet output table
Co-authored-by: Tobias Kuhlmann <tobias.kuhlmann@clabs.co>
* Remove eksportisto 8 deployment config.
* remove eksportisto deployment sensitive account configs
* Rename legacy eksportisto helm charts folder.
* Fix eksportisto helm release existence test.
The previous implementation would incorrectly match for
'rc1-eksportisto-1' on 'rc1-eksportisto-11'.
* Fix service account e-mail fetching.
As it is, more than one account might match given a name. Change the
filtering from "displayName:$QUERY" to "displayName<=$QUERY AND
displayName>=$QUERY", as suggested by gcloud's documentation
(https://cloud.google.com/sdk/gcloud/reference/topic/filters).
* Fix service account existence check.
Previously false positives were present due to prefix matches.
* Fix broken get-attestations test (#8790)
* Add network alias for global node flag (#8549)
### Description
- Add aliases to `--node` for ease of typing
* Add isFunded construct to ReleaseGold (#8751)
### Description
- Removes requirement for funding in `initialize`
- Adds storage boolean `funded`
- Adds view function `isFunded`
### Tested
Unit tests
### Related issues
- Fixes #8486
### Backwards compatibility
With usage of init and accounting for already deployed grants which will be updated
* Adds new script for deploying/updating sdks (#8739)
* Fix sourcify verification full_match path (#8800)
* Fix attestation bot (#8782)
### Description
Need to use `getAttestationStat` instead of `lookupIdentifiers` to be consistent with downstream checks. `getAttestationStat` returns all attestations for an identifier, including attempted. `lookupIdentifiers` only returns completed. This mismatch caused phone numbers to be considered valid in `getPhoneNumber()` when [this check would fail](https://github.com/celo-org/celo-monorepo/blob/master/packages/celotool/src/cmds/bots/auto-verify.ts#L137).
### Tested
Running in Baklava.
https://cauldron.pretoriaresearchlab.io/baklava-attestations
* Remove await for walletconnect connect, beta.19 (#8801)
* Add library support for CIP-40 domain hashing (#8705)
* refactor and support array types in EIP-712 encoding
* add domain and request types
* start adding tests for eip712 function with encodeType
* update encodeType to throw an error when an unrecognized type is encountered
* add array support for encodeType and typeHash test
* add basic data example and fix edge case with string encoding
* use the `keccak` alias instead of `sha3`
* handle ints and uints outside of uint256
* add Transaction example and structHash test
* add BigNumber to EIP-712 value type definition
* fix and add test for array encoding
* add Optional type to make Domain types EIP-712 compliant
* make DomainRestrictedSignatureRequest EIP-712 compatible
* add comment and compile-time test
* add EIP-712 type for SequentialDelayDomain
* fix the build and add compile test
* add zero value handling with EIP-712 objects
* add request types and remove unused zero default code
* fix a couple of linter errors
* add and use OmitIfNever to simplify declarations
* move compile tests into separate file
* format comment
* add example to type comment
* use updated SequentialDelayDomain in rate limit code
* fix input-validaton-test types
* fix phone-number-privacy-common tests
* specify default for first type param in requests
* add domainHash function for easier use of hashing
* utilize new types in odis signer and minor refactorings
* fix remaining linter errors
* make disable domain request idempotent and allow disabling previously unknown domains
* fix build error
* refactor and add EIP712 type for domain options
* modify spec to include name and version in EIP-712 message
* add EIP-712 types for domain signature request
* add EIP-712 types for quota and disable request types
* rename the publicKey field to address
* Apply suggestions from Alec's code review
Co-authored-by: Alec Schaefer <alec@cLabs.co>
* Use more description names for EIP-712 optional type functions
* fix usage of renamed field publicKey -> address
* Add option to use unpublished dependency code to the Docker build
* fix missing comma in cloudbuild.yaml
* fix dependency refrence in signer package.json
* update yarn.lock
Co-authored-by: Alec Schaefer <alec@cLabs.co>
* Bump sdks to 1.3.1, fix some deploy-sdks script issues (#8798)
* Bump sdks to 1.3.1
* cBRL contract from contracts.v5 tag
* Update Node version for ODIS (#8775)
### Description
Update Combiner to latest Firebase and Node 12. This makes it easy to automate deployment.
### Other changes
Upgrade ContractKit to latest.
### Tested
Deployed to Staging + Alfajores. Ran E2E tests.
* add support for the forno endpoint in the make-release wrapper (#8804)
Co-authored-by: Martín Volpe <volpe@clabs.co>
* Add EpochRewards wrapper for use in `celocli network:parameters` (#8651)
### Description
- Adds `wrappers/EpochRewards.ts`
- Modifies `kit.getConfig` to fetch epoch rewards config
### Tested
<img width="945" alt="Screen Shot 2021-09-20 at 2 42 07 PM" src="https://user-images.githubusercontent.com/3020995/134079746-cdf9bf6b-d824-4215-9074-2e5574dd8a77.png">
* Change default node version to 12 (#8825)
* Add erc20 genericTransfer to ReleaseGold (#8750)
### Tested
Unit tests
### Related issues
- Fixes #8698
### Backwards compatibility
Checked in CI
* Increase forno maxpeers and refactor some promises to cap parallelism (#8477)
### Description
- Increased forno maxpeers to 1500, with 70% of a cpu core reserved for light peers. Peers increased from:
```
{"ETH":100,"LES":1000,"msg":"Maximum peer count","severity":"info","timestamp":"2021-08-05T15:22:39.314023684Z","total":1100}
```
to
```
{"ETH":500,"LES":1000,"msg":"Maximum peer count","severity":"info","timestamp":"2021-08-05T15:20:48.698882556Z","total":1500}
```
So the ETH peer slot available are <maxpeers> - <light.maxpeers> cmd parameters. <light.serve> indicates the max % of cpu
time allowed for service LES requests. It is posible to have >100 if geth has multiple cores available.
- Refactor some `Promise.all` that were throwing warnings during execution due to high parallelism
* Add Result type guards (#8835)
### Description
Adds type guards for the Result type
* Set vulnerable dependency to fixed version (#8846)
### Description
Security vulnerability: https://github.com/advisories/GHSA-pjwm-rvh2-c87w
Setting dependency to fixed version that does not have vulnerability. `0.7.28` is safe according to [this thread](https://github.com/faisalman/ua-parser-js/issues/536).
* Added constitution parameters to cREAL
* Allow for CIP8 that bypasses CIP3 (#8360)
### Description
CIP8 (offchain storage) is currently implemented on top of CIP3 (offchain metadata). DApp development can be easier if additional CIP8 storage roots can be added via an onchain transaction rather than modifying the CIP3 record.
This PR introduces a new storage variable on the Accounts smart contract that can be used to bypass CIP3 for registering new CIP8 storage roots.
### Tested
New unit tests.
### Related issues
- Fixes #7848
### Backwards compatibility
Adds new storage variable and related methods, doesn't currently affect anything else. This would be a first step in deprecating CIP3.
* Reduce blockscout autoscaling max. (#8822)
* Exclude phoenix metrics in grafana cloud (#8849)
Exclude the `phoenix_.+` metrics from grafana cloud because their size and we are not using those metrics.
Co-authored-by: Donald Hutchison <donald.hutchison@clabs.co>
* Ingest GKE and AKS logs in Grafana Cloud (#8827)
### Description
Forward GKE application logs to Grafana Cloud's Loki instance through the Promtail client.
Since Grafana Cloud is the new designated central place to handle monitoring, it should have (at least) application logs to allow building dashboards, creating alerts, and troubleshooting.
The current setup is split: metrics are in or forwarded to GCP Stackdriver, as well as in Grafana Cloud, logs are (mostly) in their respective clouds.
This is a further step to unify all monitoring in Grafana; Azure and aws logs are still to be ingested.
See #8834 for AKS.
### Other changes
- Sprinkle celotool with dry runs when calling it with the `--helmdryrun` flag.
Technically this is supposed to apply to (only) Helm, but it's cleaner to also prevent some k8s and GCP edits to be made.
- Refactor some of the celotool Prometheus code that has been reused for Promtail.
### Tested
```sh
celotool deploy upgrade -e alfajores --verbose # after manually initialising
celotool deploy initial -e baklava --verbose
celotool deploy upgrade -e baklava --verbose
celotool deploy initial -e baklava --context gcp-forno-europe-west1 --verbose
```
### Related issues
- Relates to https://github.com/celo-org/data-services/issues/70
### Backwards compatibility
No breaking changes.
### Documentation
N/A
* Changing cREAL parameters
* Update packages/protocol/releaseData/initializationData/creal_deploy.json
* Fix import of fixidity library in EpochRewards.ts (#8857)
### Description
In #8651, a new `EpochRewards.ts` is added, which uses the following import.
https://github.com/celo-org/celo-monorepo/blob/3ad49d73e43779544475658e7e6f32f3f326bf8b/packages/sdk/contractkit/src/wrappers/EpochRewards.ts#L1
Instead of `src` it should be `lib`, otherwise Node scripts that depend on contractkit can fail.
* Fix bug in signature parsing for EIP712TypedData (#8836)
## Description
When parsing EIP712 signed typed data in `SignatureUtils.recoverEIP712TypedDataSigner`, we break and return a value as soon as a signature can be parsed. The issue is that 1) we do not serialize signatures consistently (examples from the monorepo: `SignatureUtils.serializeSignature` serializes as VRS, but `LocalSigner.signTypedData` returns an RSV-serialized sig) 2) some small percentage of signatures can be parsed as both RSV & VRS signatures, in which case the order of the parsing matters as long as we do not check against the expected signer.
Given that the usage in `offchain-data-wrapper` is relatively new (and breaking changes shouldn't be too disruptive), the proposed fix splits up `recoverEIP712TypedDataSigner` into two (VRS & RSV-specific, so the caller can choose to explicitly call these if it's necessary to "guess" an address), while modifying the `verify` function to be in line with how the existing message parsing `verifySignature` function works -- i.e. boolean that suppresses errors but works regardless of RSV/VRS serialization.
This is a breaking change in that `recoverEIP712TypedDataSigner` has been deprecated in favor of two functions.
### Other changes
- wraps `SignatureUtils.verifyEIP712TypedDataSigner` in a try/catch to ensure that it always returns a boolean (and no longer propagates the error from the recovery step). This is in line with the other signature parsing/verification functions.
- adds unit tests for `recover & verify`: these are pretty dumb and based on hardcoded examples in order to not add the CK/LocalWallet dependencies to the utils package -- open to thoughts on this though, if you think it's worthwhile to randomize PKs but incorporate these dependencies)
### Testing
- [x] unit tests for `recover & verify` against a hard-coded edge case that was failing
- [x] existing unit tests for `offchain-data-wrapper` pass
- [x] programmatically tested that previous issue with certain AS codes causing this is resolved for large ranges of codes
- [x] TODO deploy to alfajores AS + make sure no changes to existing flow (also tried with contrived code examples that would have thrown the `Invalid signature` error, but now throw "Invalid code" as we want)
Closes #8812
_Edit: further notes for future documentation:_
### Notes on implementation choices in `offchain-data-wrapper` and future TODOs here:
Initially @nambrot and I thought it would make sense to double down on the usage of just VRS-serialization. This PR diverges from that initial thought due to the following:
- in `PublicSimpleAccessor.sign` we are actually using the `wallet.signTypedData` method which returns an RSV-signature (we can switch this to using the `CK.signTypedData` + `SignatureUtils.serializeSignature`, but this increases the scope of this PR)
- we could just use the RSV recover function, but this moves away from compatibility with serializeSignature (VRS)
- (even within the wrapper code, we use both `wallet.signTypedData` and `serializeSignature` (in the tests), so standardizing should take a more rigorous comb through of everything here IMO)
The current implementation is slightly less efficient than before, but should not suffer from the same edge case that this PR fixes (except in the case that the `guessedSignature` must still be used, which seems unavoidable). Notes on this:
- we repeat signature recoveries here (i.e. use the `verifyEIP712TypedDataSigner`) which is less efficient (on the order of a few milliseconds in total)
- if we don’t have a single standard signature, then moving the signature-guessing logic up will either 1) reintroduce the parsing edge case or 2) make it less readable & duplicates logic IMO (try to parse both, check equality against up to 2 parsed sigs)
I ultimately opted for minimizing the amount of signature standardization across the module for now and eliminating the possible edge case, and would be slightly in favor of splitting out signature standardization across the offchain-data-wrapper into a separate ticket for the future.
TODOs
- [x] [immediate] create a ticket for the below and link here -> created issue #8862 (assigned to Applications initially)
* Add signature verification code for CIP-40 requests (#8859)
* add verification and signing example for signature request
* add additional test cases
* add verification functions for the other two request types
* generalize tests to cover all signature verifier functions
* add comments to the verify and build typed data functions
* update dependency graph
* Added migrations cREAL
* Added integration tests and migrations config
* Emilhotkowski/domain versioning2 (#8833)
* update signer dependency
* v1 of versioning and quota for domains
* v1 of versioning and quota for domains
* Added transaction to Domains
* Corrected imports
* Change migration
* Added signing method
* PR Changes
* Added simple test
* Corrected test for domain service
* PR changes v2
* Lint fix
* lint
* Update packages/protocol/test/common/integration.ts
* Removed json
* retigger CI
* Changed base RELEASE_TAG
* Added release5-report.json
* Migrate to typescript@4.4.3 (#8789)
* new line
* Added CR6 script
* Don't rely on bignumber.js in @celo/base (#8923)
### Description
There was this small exception to the objective of `@celo/base` not having any dependencies
* Add apps panel variables (#8936)
* Add apps panel variables
* Add some missing projects
* Refactor TwilioSmsProvider into two separate modules (messaging, verify) (#8871)
## Description
Closes #8755
Builds on the earlier [configurable verify PR](https://github.com/celo-org/celo-monorepo/pull/8783).
Note: the mocking is SUPER ugly and feels almost useless here; very open to ideas on structuring the tests better, but I think this is another arg in favor of refactoring the SMS module cc @emilhotkowski 😅
## Changes
- Splits `TwilioSmsProvider` into two providers: `TwilioVerifyProvider` and `TwilioMessagingProvider`; providers can now be configured by country `SMS_PROVIDERS_X=twiliomessaging,twilioverify,...`, and Verify/Messaging will be by default randomized (instead of preferring Verify as previously). (I'm really not attached to the env var names, if someone has a better idea that's less clunky, I'm all 👂's!)
- `twilio` provider in env variable setting is now syntactic sugar for both `twilioverify` and `twiliomessaging`, to keep backwards compatibility of env vars
- Adds unit tests for the `sendSms` function (where the above changes are located)
- Mocking of the twilio module to make this test possible ^
- Adds a test script to really test out the `sendSms` function (i.e. using the real twilio module), usable by adding the `TEST_SMS_RECIPIENT` parameter in `.env.development` (and making sure the necessary twilio SID + AUTH vars are also set there, just as needed for running the AS locally)
- Splits out `SmsFields` from `AttestationModel` interface to make it possible to write the above two types of tests. It isn't a super clean break right now (makes sense to revisit with the full refactor of the SMS module), but it roughly separates into: `SmsFields` = fields required to produce an actual SMS, `AttestationModel` = metadata regarding production of the SMS (i.e. provider, attempt) + underlying data model
Relevant docs PR: https://github.com/celo-org/docs/pull/211
## Testing
- [x] used the two test scripts
- [x] tested this locally
- [x] deployed to Alfajores
- [x] tested verification flows on Alfajores -- able to switch on/off for DE (my phone number), other flows work as expected
* return veriifcation as false for unauthenticated domains (#8937)
Co-authored-by: Emil Hotkowski <emil.hotkowski@gmail.com>
* Fixed typos
* contractkit now knows BRL exists
* Require that slashing reporters are accounts (#8854)
### Description
Slashing is incentivized with a reward of locked CELO going from the slashee to the reporter. Locked CELO is tied to accounts from the Accounts core contract, so we require that reporters have created an account.
### Other changes
Reworded a description string for an unrelated test.
### Tested
Unit tests.
### Related issues
- Fixes #8828
### Backwards compatibility
This change doesn't change storage layouts or function signatures.
* Eksportisto deployment v3.
* Fixes typescript versioning issue (#8927)
* Update env vars for blockscout apps links (#8956)
* Update docker images and testnet package (#8959)
### Description
- Updated celotool image to be able to use to deploy `testnets` and run `mycelo` (added dependencies, helm-charts to image)
- Fixed docker image dependencies
- Fixed loadtests helm chart when using mycelo genesis (too big f…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
To enable metrics we had a small bug that get fixed with this PR.
Tested
Tested in Alfajores