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

feat(protocol): put automata dcap v3 ra behind proxy #16867

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

adaki2004
Copy link
Contributor

As per request. #16864 (comment)

@dantaik
Copy link
Contributor

dantaik commented Apr 26, 2024

and who is the owner to perform upgrade? The idea was deploy non-upgradable contract to use address manager to switch to a new verifier if needed.

@adaki2004
Copy link
Contributor Author

and who is the owner to perform upgrade? The idea was deploy non-upgradable contract to use address manager to switch to a new verifier if needed.

Owner to upgrade i guess is the proxy owner.
But good question, i let it be answered by John, dont know who shall maintain this.

Copy link

openzeppelin-code bot commented Apr 26, 2024

feat(protocol): put automata dcap v3 ra behind proxy

Generated at commit: be80e1d0de7d4f0738224cefedc67c5c8137ab80

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
2
0
5
41
50
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@dantaik
Copy link
Contributor

dantaik commented Apr 26, 2024

If this contract is stateless, then maybe we just deploy a new version.

@adaki2004
Copy link
Contributor Author

adaki2004 commented Apr 26, 2024

If this contract is stateless, then maybe we just deploy a new version.

In our current design:
A: deploy a new and upgrade the lookup in AddressManager
or
B: upgrade

both can work. I have no strong opinion, maybe B is more handy and easier (?). Please you decide.

@dantaik
Copy link
Contributor

dantaik commented Apr 29, 2024

@johntaiko @smtmfft

The reason I think we should keep this contract not upgradeable is to ensure we can sync with the official repo's code easily, otherwise, the official repo is not upgradeable and ours is, then we always have to manually copy code and make necessary changes to our copy to ensure it works.

@dantaik
Copy link
Contributor

dantaik commented Apr 29, 2024

@johntaiko @smtmfft

The reason I think we should keep this contract not upgradeable is to ensure we can sync with the official repo's code easily, otherwise, the official repo is not upgradeable and ours is, then we always have to manually copy code and make necessary changes to our copy to ensure it works.

Given that this verifier has state data, I'm OK with upgradeability

@dantaik dantaik added this pull request to the merge queue Apr 29, 2024
Merged via the queue into main with commit 1282113 Apr 29, 2024
7 checks passed
@dantaik dantaik deleted the put_automata_behind_proxy branch April 29, 2024 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants