Lightweight project implementation #83
Conversation
|
|
||
| export default class AppProject extends Project { | ||
| export default class AppProject extends BasePackageProject { |
There was a problem hiding this comment.
I think we can use just PackageProject
There was a problem hiding this comment.
Hmm I actually prefer to make it clear which are base classes, and which are leaves that can be actually instantiated.
| if (!contractName) contractName = contractClass.contractName | ||
| if (!packageName) packageName = this.name | ||
| if (!_.isEmpty(initArgs) && !initMethod) initMethod = 'initialize' |
There was a problem hiding this comment.
I think this check was already performed at an upper level, isn't it? If so, do you think it still make sense to have it duplicated?
There was a problem hiding this comment.
It may be done at an upper level on the CLI, but since we may want to start promoting usage of the lib directly, we need to have it here as well.
There was a problem hiding this comment.
Actually I scanned through the NetworkAppController, scripts/create, and commands/create, and didn't found it. It could be that I already removed it when making the change to Projects.
| async _tryInitializeProxy(proxy, contractClass, initMethodName, initArgs) { | ||
| if (!initMethodName) return; | ||
| if (this.txParamsInitializer.from === this.txParamsAdmin.from) { | ||
| throw Error(`Cannot initialize the proxy from the same address as its admin address. Make sure you use a different 'from' account for the initialization transaction params.`) |
There was a problem hiding this comment.
I see why we need to do this but it will be completely weird for the users and I'm sure they won't understand it at once. If we move the initialization method call to the constructor, this shouldn't be a problem anymore, am I right?
There was a problem hiding this comment.
Indeed, that's why I was interested in pushing that change; we should work on it ASAP. Still, keep in mind that the user would still need a 2nd account to interact with the proxy after creating it, but at least the Project can work with a single account.
| await proxy.changeAdmin(newAdmin) | ||
| log.info(`Proxy admin changed to ${newAdmin}`) | ||
| return proxy | ||
| } |
There was a problem hiding this comment.
In general, I'd love to expose an interface that abstracts you from the idea of having a proxy under the hood, even more if are planning to deal with non-upgradeable instances. Honestly, I don't have better options for naming now, we can think more about them later together
| @@ -18,13 +17,14 @@ const assertions = helpers.assertions | |||
| const assertRevert = helpers.assertRevert | |||
|
|
|||
| // model objects | |||
| import Proxy from './proxy/Proxy' | |||
There was a problem hiding this comment.
I think we don't need to import this one right?
There was a problem hiding this comment.
It's exported in L40 of this file
spalladino
force-pushed
the
feature/lightweight-projects
branch
from
8885b03
to
dca8082
Compare
- Rename Project to PackageProject - Add ProxyProject behaviour - Minor changes in projects and baseapp
- Add create method
- Move buildCallData and callDescription out of BaseApp - Add new sendDataTransaction method to Transactions object, to estimate gas of a manually crafted transaction - Use sendDataTransaction when initializing a non-upgradable instance
- Add methods to proxy to support deploying and managing
Since it is an abstract class
spalladino
force-pushed
the
feature/lightweight-projects
branch
from
dca8082
to
7044ed1
Compare
|
@facuspagnuolo could you please re-review? I've only rebased on top of |
* Add tests for proxy management in app project - Rename Project to PackageProject - Add ProxyProject behaviour - Minor changes in projects and baseapp * Move Proxy from utils to models - Add create method * Refactor buildCallData methods into an ABIs utils - Move buildCallData and callDescription out of BaseApp - Add new sendDataTransaction method to Transactions object, to estimate gas of a manually crafted transaction - Use sendDataTransaction when initializing a non-upgradable instance * Initial implementation of simple project - Add methods to proxy to support deploying and managing * Rename PackageProject to BasePackageProject Since it is an abstract class
* Add tests for proxy management in app project - Rename Project to PackageProject - Add ProxyProject behaviour - Minor changes in projects and baseapp * Move Proxy from utils to models - Add create method * Refactor buildCallData methods into an ABIs utils - Move buildCallData and callDescription out of BaseApp - Add new sendDataTransaction method to Transactions object, to estimate gas of a manually crafted transaction - Use sendDataTransaction when initializing a non-upgradable instance * Initial implementation of simple project - Add methods to proxy to support deploying and managing * Rename PackageProject to BasePackageProject Since it is an abstract class
Provides an implementation named SimpleProject for a lib/Project that can manage proxies without setting up a Package or App contracts at all. Note that SimpleProject requires to be provided with two sender accounts, one for managing proxies and one for initializing them, since the TransparentProxy pattern makes it impossible for the admin of the proxy to invoke
initialize(or any other function of the underlying instance).Fixes #70