[ETHEREUM-CONTRACTS] [DEVX] simplify SuperApp registration
d10r opened this issue · 3 comments
The registration key doesn't serve any purpose (anymore) after switching to an expire date based logic instead of one-time keys.
As a devx improvement, we could now simplify the logic to this:
registerApp(configWord)
for registration through the constructor, invoked by a deployer.registerApp(app, configWord)
for registration through a factory contract.
Overloading isn't an issue in the Solidity API (it's a devx issue only with e.g. ethers.js).
registerAppWithKey
and registerAppByFactory
can be kept for a while marked as deprecated (for backwards compatibility) and later removed. But docs and examples shouldn't use them anymore.
Internally (in governance contract and scripts) we can either keep the logic as is (declaring e.g. the empty string as default key) or simplify the code. It could be simplified to just whitelist deployers, regardless of EOA or contract.
If we want to keep the distinction and associated constraints (EOA: call only from constructor), that could be achieved implicitly by checking if the msg.sender is an EOA or contract in registerApp
.
https://github.com/superfluid-finance/protocol-monorepo/wiki/About-App-Registry updated accordingly:
About APP_RULE_REGISTRATION_ONLY_IN_CONSTRUCTOR:
This is called the APP_RULE_REGISTRATION_ONLY_IN_CONSTRUCTOR rule. The intent for this rule is to prevent a contract from upgrading itself to Super app after the fact. Retroactively, APP_RULE_REGISTRATION_ONLY_IN_CONSTRUCTOR is more on the nanny-state rule side, since it tries to protect users from malfunctioning or malicious Super apps.
About app registration key:
The design purpose of the key is to allow whitelisting process to have "finer control" over the same EOA.
--
After some discussion, the decision is to:
deprecate APP_RULE_REGISTRATION_ONLY_IN_CONSTRUCTOR nanny rule and registration key usage.
--
However, in the long run, the goal is still to depreciate app registration whitelisting process.
now behaves like this for existing authorizations:
- registered factories shall keep working
- existing registration keys shall keep working
new authorizations:
- are created using the ops-script
gov-authorize-app-deployer.js
- deployer account can be EOA or contract
registerApp
is used in all cases- if an
app
argument is NOT provided, it's a self-registration by a SuperApp. In this case the tx.origin (EOA) needs to be authorized - if an
app
argument is provided, the msg.sender needs to be authorized (may be a contract)
- if an
This covers all previous possibilities, but makes less assumptions. E.g. also allows registration of upgradable SuperApps without "abusing" a registration method not meant to be used that way.
All new authorizations are mapped to the gov config key org.superfluid-finance.superfluid.appWhiteListing.registrationKey
, setting the key to "k1". That's not pretty, but we decided it's not worth the complication (e.g. breakage of existing authorizations) to change this again. Focus was on simplifying the public API and devx for SuperApp devs.
The expiration date is now set to (effectively) unlimited by default (to be more precise, to 2^64 - 1). It can still be set to any other value if the optional env var EXPIRATION_TS
is set.