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.

Sign up for GitHub

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

Jump to bottom

Make AirnodeRrpV0 and RequesterAuthorizerWithErc721 addresses optional #1755

Merged
merged 14 commits into from
May 26, 2023
Merged

Make AirnodeRrpV0 and RequesterAuthorizerWithErc721 addresses optional #1755

merged 14 commits into from
May 26, 2023

Conversation

dcroote
Copy link
Contributor

@dcroote dcroote commented May 1, 2023

Closes #1717

@dcroote dcroote self-assigned this May 1, 2023
@dcroote dcroote requested a review from a team May 1, 2023 07:23
Siegrift
Siegrift requested changes May 2, 2023
View reviewed changes
Copy link
Contributor

@Siegrift Siegrift left a comment

Choose a reason for hiding this comment

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

I did a quick sweep and I think we can implement this a bit more cleanly - esp. https://github.com/api3dao/airnode/pull/1755/files#r1182262839

dcroote and others added 3 commits May 2, 2023 21:19
@dcroote @Siegrift
Incorporate PR feedback
1959845 
Co-authored-by: Emanuel Tesar <[email protected]>
@dcroote @amarthadan
Remove cyclic dependency between packages
df35680 
airnode-validator depends on airnode-utilities solely for the logger in
one script

Co-authored-by: Michal Kimle <[email protected]>
@dcroote
Import references.json from dist/ of airnode-protocol
c43779b
@bbenligiray bbenligiray mentioned this pull request May 16, 2023
Merged
@dcroote
Copy link
Contributor Author

dcroote commented May 25, 2023

Who would have thought providing a few defaults would be such a pain. Anyway, this PR now allows omission of the addresses for AirnodeRrpV0 and RequesterAuthorizerWithErc721 when we've deployed those contracts on a given chain. To do this, we look up those addresses exported from @api3/airnode-protocol and throw an error if a user omits the address for a chain where we haven't deployed. This still preserves the ability to specify the address for a chain independent of whether there is a deployment and so is backwards compatible.

@dcroote dcroote requested a review from Siegrift May 25, 2023 07:11
@dcroote dcroote changed the title Default to deployed AirnodeRrpV0 if contracts omitted from config Make AirnodeRrpV0 and RequesterAuthorizerWithErc721 addresses optional May 25, 2023
Siegrift
Siegrift approved these changes May 25, 2023
View reviewed changes
Copy link
Contributor

@Siegrift Siegrift left a comment

Choose a reason for hiding this comment

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

👍 LGTM. After fixing the TS inference in validator, this should be good to go.

@dcroote dcroote merged commit deb1b35 into master May 26, 2023
@dcroote dcroote deleted the dcroote/issue1717 branch May 26, 2023 07:08
This was referenced Jun 16, 2023
Closed
Merged
@dcroote dcroote mentioned this pull request Jul 22, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siegrift Siegrift Siegrift approved these changes

Assignees

@dcroote dcroote

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make all protocol contract addresses optional
2 participants
@dcroote @Siegrift