search

0xpatrickdev / agoric-vault-collateral-proposal

CoreEval Proposal and Permits for Inter Vault Collateral Type
0 stars 3 forks source link

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

Closed dckc closed 1 year ago

dckc commented 1 year ago

moved upstream:

I wondered why this is requested:

https://github.com/0xpatrickdev/agoric-vault-collateral-proposal/blob/1aa1b7036c3cf349ea9fb342f1474b53d1cf0a3f/add-stATOM-oracles-permit.json#L12

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.

0xpatrickdev commented 1 year ago

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: https://github.com/0xpatrickdev/agoric-vault-collateral-proposal/issues/5#issuecomment-1684990253

dckc commented 1 year ago

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

0xpatrickdev commented 1 year ago

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

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

dckc commented 1 year ago

migrating upstream: