wighawag / hardhat-deploy

hardhat deployment plugin
MIT License
1.17k stars 283 forks source link

no method named "owner" on contract when deploying #477

Open mwawrusch opened 10 months ago

mwawrusch commented 10 months ago

Describe the bug When installing or upgrading through hardhat-deploy I get the following error message after the contracts have been deployed but before the facets have been installed: no method named "owner" on contract ... See full error message below. I am using a inherited version of the contract (see below).

Manually adding facets through louper worked though, and no problems in development either.

To Reproduce I can provide access to the github.

versions

Additional context

 at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async _deployViaDiamondProxy (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/helpers.ts:2476:30)
    at async Object.func (/Users/martinwawrusch/Dev/xxx/deploy/001_deploy_dekoh.ts:49:39)
    at async DeploymentsManager.executeDeployScripts (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/DeploymentsManager.ts:1226:22)
    at async DeploymentsManager.runDeploy (/Users/martinwawrusch/Dev/rxxx/node_modules/hardhat-deploy/src/DeploymentsManager.ts:1059:5)
    at async SimpleTaskDefinition.action (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/index.ts:438:5)
    at async Environment._runTaskDefinition (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat/src/internal/core/runtime-environment.ts:333:14)
    at async Environment.run (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat/src/internal/core/runtime-environment.ts:166:14)
    at async SimpleTaskDefinition.action (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/index.ts:584:32)
    at DeploymentsManager.executeDeployScripts (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/DeploymentsManager.ts:1229:19)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async DeploymentsManager.runDeploy (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/DeploymentsManager.ts:1059:5)
    at async SimpleTaskDefinition.action (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/index.ts:438:5)
    at async Environment._runTaskDefinition (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat/src/internal/core/runtime-environment.ts:333:14)
    at async Environment.run (/Users/martinwawrusch/Dev//node_modules/hardhat/src/internal/core/runtime-environment.ts:166:14)
    at async SimpleTaskDefinition.action (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/index.ts:584:32)
    at async Environment._runTaskDefinition (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat/src/internal/core/runtime-environment.ts:333:14)
    at async Environment.run (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat/src/internal/core/runtime-environment.ts:166:14)
    at async SimpleTaskDefinition.action (/Users/martinwawrusch/Dev/xxx/node_modules/hardhat-deploy/src/index.ts:669:5)
Diamond code
```
import { Diamond } from "hardhat-deploy/solc_0.8/diamond/Diamond.sol";

contract DekohDiamond is Diamond {

/// @notice This construct a diamond contract
/// @param _contractOwner the owner of the contract. With default DiamondCutFacet, this is the sole address allowed to make further cuts.
/// @param _diamondCut the list of facet to add
/// @param _initializations the list of initialization pair to execute. This allow to setup a contract with multiple level of independent initialization.
constructor(
    address _contractOwner,
    IDiamondCut.FacetCut[] memory _diamondCut,
    Initialization[] memory _initializations
) payable Diamond(_contractOwner, _diamondCut, _initializations)

{ } }

mwawrusch commented 10 months ago

To add to that:

defaultOwnershipFacet is set to false, which probably causes this.

apollox-apx commented 6 months ago

During the use of hardhat-deploy, I encountered a situation similar to issue #477. I would like to share my perspective. I believe the Diamond contract does not necessarily need to have the OwnershipFacet, as it is not part of the eip-2535 standard and thus does not require the owner method. In terms of permission management, considering its limited authority, I may not use the OwnershipFacet. Instead, I am inclined to use the AccessControlEnumerableFacet for better permission management. Providing this feedback may help improve and optimize this library for a better user experience.

Many thanks for your efforts and contributions to the community. Looking forward to seeing this issue resolved and to the ongoing progress and development of your project.

apollox-apx commented 6 months ago
image

https://eips.ethereum.org/EIPS/eip-2535#ownership-and-authentication