-
Notifications
You must be signed in to change notification settings - Fork 23
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
Support encryptor allowlist #306
Support encryptor allowlist #306
Conversation
Codecov Report
@@ Coverage Diff @@
## monorepo #306 +/- ##
============================================
- Coverage 88.25% 87.86% -0.40%
============================================
Files 33 33
Lines 2409 2430 +21
Branches 205 205
============================================
+ Hits 2126 2135 +9
- Misses 254 266 +12
Partials 29 29
|
e5ac346
to
ead75e9
Compare
ead75e9
to
d12d203
Compare
GlobalAllowListInterface, | ||
} from "../GlobalAllowList"; | ||
|
||
const _abi = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a difference between this _abi
and packages/shared/abis/GlobalAllowList.json
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is code generated by typechain
, the difference includes generated (inferred) types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok cool, so as one changes, the other will be automatically kept up to date by typechain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, although if there's an incoming PR that will integrate nucypher-contracts >= v0.5.0
do we want to wait for that first?
Yes, I will add this information to the PR description. |
}; | ||
|
||
const MUMBAI: Contracts = { | ||
SUBSCRIPTION_MANAGER: '0xb9015d7b35ce7c81dde38ef7136baa3b1044f313', | ||
COORDINATOR: '0x8E49989F9D3aD89c8ab0de21FbA2E00C67ca872F', | ||
GLOBAL_ALLOW_LIST: '0x7b521E78CFaf55fa01433181d1D636E7e4b73243', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be careful here. Both tapir
and lynx
use Mumbai but have different GlobalAllowList contracts:
- Lynx:
0x7b521E78CFaf55fa01433181d1D636E7e4b73243
- Tapir:
0x5Ff235481bbDBF5dFCC908d987506FA6a8696678
How would this resolution work currently? Keying by "domain" is better than solely by blockchain/chain_id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will address post #314 changes
d12d203
to
b09a845
Compare
b09a845
to
fac82b6
Compare
6eb9ee5
to
52e0d29
Compare
Type of PR:
Required reviews:
What this does:
Issues fixed/closed:
registerEncrypters
fromtaco
API #324Why it's needed:
Notes for reviewers:
string
usage withChecksumAddress
usage where possibleisAuthorizedEncrypter
added in Refactor packages APIs #301ethers-typechain
directory contains auto-generated code and need not be reviewed