encodeFunctionData for `multiInit` function inside `DiamondMultiInit` library fails

[paradoux]

Hi @mudgen !

First of all thanks a lot for this implementation example 🙏

I'm hitting a very strange error when trying to deploy & initialise my Diamond along with multiple facets in one transaction.

The function encodeFunctionData used to create the calldata passed into the diamondArgs initCalldata field returns the following error: Error: no matching function (argument="name", value="multiInit", code=INVALID_ARGUMENT, version=abi/5.7.0)

Here is a part of the deploy flow:

  // Just above I deploy my facets & add them to the facetCuts array like in your script 
  // I also use encodeFunctionData to create the calldata needed to create initializeErc20Call, initializeFacetBCall & initializeFacetCCall and it works fine

  const diamondMultiInitFactory = await ethers.getContractFactory("DiamondMultiInit");
    const diamondMultiInit = await diamondMultiInitFactory.deploy();
  await diamondMultiInit.deployed()
  let multiInitCallData = diamondMultiInit.interface.encodeFunctionData('multiInit', [
    [erc20.address, facetB.address, facetC.address],
    [initializeErc20Call, initializeFacetBCall, initializeFacetCCall]
  ])

  const diamondArgs = {
    owner: myOwner.address,
    init: diamondMultiInit.address,
    initCalldata: multiInitCallData
  }

  const diamond = await diamondFactory.deploy(facetCuts, diamondArgs)
  await diamond.deployed()
  console.log('Diamond deployed:', diamond.address)

And when I look into the interface of the deployed DiamondMultiInit library I get:

 interface: Interface {
    fragments: [ [ErrorFragment], [ErrorFragment], [ErrorFragment] ],
    _abiCoder: AbiCoder { coerceFunc: null },
    functions: {}, // Empty is weird
    errors: {
      'AddressAndCalldataLengthDoNotMatch(uint256,uint256)': [ErrorFragment],
      'InitializationFunctionReverted(address,bytes)': [ErrorFragment],
      'NoBytecodeAtAddress(address,string)': [ErrorFragment]
    },
    events: {},
    structs: {},
    deploy: ConstructorFragment {
      name: null,
      type: 'constructor',
      inputs: [],
      payable: false,
      stateMutability: 'nonpayable',
      gas: null,
      _isFragment: true
    },
    _isInterface: true
  }

Now I've tried to play around a bit with the multiInit function to investigate if there was a problem with the arguments I was passing etc and I ended up realising that when I change this multiInit function to a pure or view function (so commenting out the LibDiamond.initializeDiamondCut(_addresses[i], _calldata[i]); code) then I no longer get this no matching function error. And the interface looks correct this time:

interface: Interface {
    fragments: [ [ErrorFragment], [FunctionFragment] ],
    _abiCoder: AbiCoder { coerceFunc: null },
    functions: { 'multiInit(address[],bytes[])': [FunctionFragment] }, // Now I see the multiInit
    errors: {
      'AddressAndCalldataLengthDoNotMatch(uint256,uint256)': [ErrorFragment] // Here I weirdly no longer see InitializationFunctionReverted & NoBytecodeAtAddress errors now, they were present above..
    },
    events: {},
    structs: {},
    deploy: ConstructorFragment {
      name: null,
      type: 'constructor',
      inputs: [],
      payable: false,
      stateMutability: 'nonpayable',
      gas: null,
      _isFragment: true
    },
    _isInterface: true
  }

Also worth noting it behaves correctly when I turn the library into a normal contract, so maybe it's related ?

I can provide the full deploy flow script & facets code if that helps 👍

Thanks a lot in advance for your help :))

[mudgen]

Also worth noting it behaves correctly when I turn the library into a normal contract, so maybe it's related ?

Yes, I think it is related. I don't have an answer for this but I am interested to know if you work this out.

Maybe await ethers.getContractFactory("DiamondMultiInit"); does not work correctly for Solidity libraries. Or in general maybe the hardhat code you working with is designed for smart contracts and not Solidity libraries. It is worth investigating.

[paradoux]

Hey @mudgen ! So I've dug a bit into this. Under the hood, await ethers.getContractFactory is just retrieving the abi from the artifacts and generates a ContractFactory instance (from ethers.js class ContractFactory) with it. From this you can call .deploy() etc. After checking the abi generated by the compiler in artifacts, it seems the multiInit function is not present when this DiamondMultiInit is a library. And it is present when it's a contract. Hence the error Error: no matching function (argument="name", value="multiInit", code=INVALID_ARGUMENT, version=abi/5.7.0) So it seems the underlying problem is in the compiling of DiamondMultiInit by hardhat and this absence of the multiInit function in the resulting abi. I'm wondering which compiler did you use to make it work back then if not the hardhat one ?

[mudgen]

@paradoux Thanks for discovering this.

I'm wondering which compiler did you use to make it work back then if not the hardhat one ?

Perhaps the Solidity compiler can be used to generate the necessary ABI file.

Also, it seems to me that you could define a Solidity interface and use the ABI of the interface.

[paradoux]

Hey @mudgen !

Also, it seems to me that you could define a Solidity interface and use the ABI of the interface.

So I think with ethers.js you can create an interface but you need the ABI to create such an instance, so if the generated ABI is missing a function you won't have it in the created interface. (See this part of the doc). You can also retrieve the interface from a ContractFactory, but that means you've already created that Contract Factory, so again using this ethers.getContractFactory beforehand (See this part of the doc).

Perhaps the Solidity compiler can be used to generate the necessary ABI file.

So that's what I did (using solc --abi on the cli) and got the same result. After some digging I'm wondering if this behaviour isn't made on purpose actually. Have a look at this discussion on the ethereum/solidity repo. Did you use a version older than 0.5.6 when you used the solidity compiler ? If we're prevented from having the multiInit function in the ABI to prevent any potential direct call, could a solution be that if you know you'll initialize multiple facets you should just implement the multiInit function inside LibDiamond and call it within initializeDiamondCut instead of (bool success, bytes memory error) = _init.delegatecall(_calldata); line 181 ? Thanks again for your help!

[mudgen]

@paradoux Do you mean make multiInit an internal function within LibDiamond? Yes, that is a good idea.

Why are you trying to make DiamondMultiInit a Solidity library? As you said, it works as a normal contract. So why not use it as a normal contract?

[paradoux]

Hey @mudgen !

@paradoux Do you mean make multiInit an internal function within LibDiamond? Yes, that is a good idea.

Yes exactly, cool I'll try to do this then 👍 Thanks!

Why are you trying to make DiamondMultiInit a Solidity library? As you said, it works as a normal contract. So why not use it as a normal contract?

Well I think you're trying not me 😄 Your implementation example details the following inside the DiamondMultiInit.sol

// This Solidity library is deployed because it contains an external function.
// This is deployed as a Solidity library instead of as regular contract because deployed Solidity libraries 
// cannot be deleted. If this was a contract then someone could call multiInit directly on the contract
// with a regular external function call in order to delegatecall (via LibDiamond.initializeDiamondCut) 
// to a function that executes self destruct.

library DiamondMultiInit {    

    // This function is provided in the third parameter of the `diamondCut` function.
    // The `diamondCut` function executes this function to execute multiple initializer functions for a single upgrade.

    function multiInit(address[] calldata _addresses, bytes[] calldata _calldata) external {        
        if(_addresses.length != _calldata.length) {
            revert AddressAndCalldataLengthDoNotMatch(_addresses.length, _calldata.length);
        }
        for(uint i; i < _addresses.length; i++) {
            LibDiamond.initializeDiamondCut(_addresses[i], _calldata[i]);            
        }
    }
}

I think this was changed from a Contract to a Library in this commit back in 2022 :))

Let me know if I can help in any way 👍

[mudgen]

Well I think you're trying not me. Your implementation example details the following inside the DiamondMultiInit.sol

Oh, you are right! Thanks.

[xykota]

Well I think you're trying not me. Your implementation example details the following inside the DiamondMultiInit.sol

I have been stuck on this very issue as well. In short, I was using DiamondMultiInit as a library to prevent self destruct like your comments say. Is your new recommendation to use a contract, @mudgen?

Thanks for sharing your diamond code, it's been of great help.

[mudgen]

Yes, use contract. I changed the code in the repo from library to contract.