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

upstream: namesByAddressAdmin is excess authority for the flux aggregator contract? #4

Closed
dckc opened this issue Aug 3, 2023 · 4 comments

Comments

@dckc
Copy link
Collaborator

dckc commented Aug 3, 2023

moved upstream:


I wondered why this is requested:

"namesByAddressAdmin": "priceFeed",

namesByAddressAdmin is used by fluxAggregatorContract.js to "reserve" the depositFacet in the NameHub under each oracle address to make sure that during bootstrap, when invitations are sent to oracle operators, the deposits wait until those operators have created their smart wallets rather than throwing.

https://github.com/Agoric/agoric-sdk/blob/9ffd0970103383baf8b43b59b464a3467fb65d58/packages/inter-protocol/src/price/fluxAggregatorContract.js#L136-L137

But it's a lot of authority. It's sufficient authority to redirect deposits to any address. And if we can manually/socially ensure that the oracle operator smart wallets are in place before this proposal, it's not necessary.

Ideally, the "reserve" step would only be done in bootstrap, and the flux aggregator contract itself would only send deposits.

Given the way the contract is, this proposal has to have the namesByAddressAdmin authority. But we should consider fixing the contract before making this proposal in production.

@dckc dckc changed the title namesByAddressAdmin is excess authority for the flux aggregator contract? namesByAddressAdmin is excess authority for the flux aggregator contract? (NOT critical) Aug 3, 2023
@dckc dckc changed the title namesByAddressAdmin is excess authority for the flux aggregator contract? (NOT critical) namesByAddressAdmin is excess authority for the flux aggregator contract? Aug 3, 2023
@0xpatrickdev
Copy link
Owner

0xpatrickdev commented Aug 18, 2023

In this commit in this branch, I removed these 2 powers (cc #5). The build files in #11 use this fork/branch, so from our local testing it would seem things work as expected without these powers.

We can maybe mark this closed since #11 is merged in. Also can leave it open until the changes land in agoric-sdk via pr(s).

Moved to relevant issue: #5 (comment)

@dckc

This comment was marked as off-topic.

@0xpatrickdev
Copy link
Owner

removed which 2 powers? this issue is about 1 power: namesByAddressAdmin

I posted in the wrong issue, comment is only relevant for #5

@dckc
Copy link
Collaborator Author

dckc commented Aug 21, 2023

@dckc dckc closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2023
@dckc dckc changed the title namesByAddressAdmin is excess authority for the flux aggregator contract? upstream: namesByAddressAdmin is excess authority for the flux aggregator contract? Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants