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

ODIS 2.0.0 #9693

Merged
merged 229 commits into from
Oct 27, 2022
Merged

ODIS 2.0.0 #9693

merged 229 commits into from
Oct 27, 2022

Conversation

alecps
Copy link
Contributor

@alecps alecps commented Jul 21, 2022

Description

  • Implements CIP40, which extends ODIS to support "domain" specific queries. A domain is a struct which specifies rate limiting information and uniquely identifies the context of the secret to be hashed. This enables granular rate limiting with the goal of hardening passwords for use as encryption keys in the PEAR Account Recovery protocol, as well as other future use cases.
  • Adds support for a new on-chain payment based rate limit to ODIS, to enable more seamless onboarding in the Federated Attestations identity protocol (ASv2). For sequence diagrams and technical details on how ODIS fits into ASv2, please refer to the ASv2 User Flows doc.
  • This is a major refactor of ODIS that introduces a new class structure (+ dependency injection) to maximize code reuse and modularity across both the ASv2 and PEAR use cases. The following diagram gives a rough illustration of how the main classes fit together.

Other changes

  • Adds io-ts type safety checks for all incoming requests and responses throughout the codebase
  • Adds support for multiple key versions and key rotations
  • Adds support for ODIS to be queried by web clients
  • Deletes legacy matchmaking code from the combiner, common package, and SDK
  • Adds support for querying a user's remaining quota (based off on-chain payments) to both the combiner and identity SDK

Tested

  • Adds integration tests that utilize in-memory nodes and databases to easily test the system's end-to-end behavior without requiring a deployment.
  • Adds unit tests for critical rate limiting, key versioning and monitoring functionality
  • Enforces new code coverage requirements in CI
  • DOES NOT add new e2e tests. These will be added in a subsequent PR that will be written while deploying and testing against our staging environment.

Backwards compatibility

  • The Signer is backwards compatible, but the Combiner is not. Signers must be upgraded before the Combiner.
  • SDK changes are not backwards compatible with old versions of ODIS. The SDK should be released after ODIS is upgraded.

Documentation

  • Signer README is updated with key-rotation instructions
  • All other relevant documentation linked above
  • There's an ongoing epic to develop client-facing tooling and documentation for ASv2

@alecps alecps requested review from eelanagaraj and removed request for a team and jcortejoso October 21, 2022 17:02
eelanagaraj and others added 2 commits October 21, 2022 14:20
* Add separate enable config value for legacy PNP signer endpoint

* Driveby change: remove unnecessary jest line from test utils

* Add legacy enabled param to celotool and helm charts

* Remove rpc-wallet dep
* enable cross origin resource sharing from any domain so odis can be interacted with from web apps

* explictly enable OPTIONS (and other methods)

* use arrow function as its required by lint config
Copy link
Contributor

@nategraf nategraf left a comment

Choose a reason for hiding this comment

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

I love the huge improvements to ODIS being made in this PR. This really is the culmination of a lot of work to improve the quality, safety, and maintainability of this service. Very excited to have this PR merged at last!

I didn't have time to do the kind of through review I usually like to do, but I did leave some comments where I could. I had a number of comments about minor issues around coding practices. I think it's time this PR be merged so the intention is not to require refactoring. I decided to leave the comments anyway because it might be useful for future work.

I did have a couple of questions around quota accounting. In particular, the fail open mechanism is still something I understand pragmatically, but always gives me pause. I left a thought on whether we could install a mitigation for by having the system "circuit break" at some number of failed open requests.

I think we may have talked about this before, but is the intention with the legacy and on-chain payment versions of quota accounting to shut down the legacy version in the near future? If so, I wish you god speed in doing so.

alecps and others added 6 commits October 24, 2022 09:53
Co-authored-by: Aaron DeRuvo <aaron@clabs.co>
Co-authored-by: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com>
Co-authored-by: isabellewei <isabelle@clabs.co>
* 2.3.0  (#9944)

* 2.3.0 Beta 1

* catch require module not found errors   (#9949)

* catch require module not found errors :)

* Beta 2

* 2.3.0 gold

* Fix bug with shouldFailOpen param flag

* Tiny nits

* Fix merge errors with OdisPayments

* Run prettify

* Fix CLI tests

Co-authored-by: Aaron DeRuvo <aaron@clabs.co>
* deletes all matchmaking code

* deletes matchmaking sdk code

* fixes error
eelanagaraj and others added 5 commits October 25, 2022 14:02
* Rename combinePartialBlindedSignatures -> combineBlindedSignatureShares

* Bump line coverage to 80 and remove TODO

* Small error handling additions

* Change distribute return type to void

* Fix CI issues with flake tracking
@alecps alecps merged commit 6a146db into master Oct 27, 2022
@alecps alecps deleted the odisCip40 branch October 27, 2022 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status:  Closed
Development

Successfully merging this pull request may close these issues.

5 participants