Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERC777 fails to deploy #4126

Closed
chriscrutt opened this issue Mar 20, 2023 · 8 comments
Closed

ERC777 fails to deploy #4126

chriscrutt opened this issue Mar 20, 2023 · 8 comments

Comments

@chriscrutt
Copy link

chriscrutt commented Mar 20, 2023

edit: added a lot more information

When I write the simplest of ERC777 contracts, it fails on deployment for:

  1. Remix VM (Merge, London, and Berlin) (it works for Mainnet, Sepolia, and Goerli)
  2. MetaMask when connected to Ganache on Remix
  3. Ganache with Truffle

Code taken from here

// contracts/GLDToken.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC777/ERC777.sol";

contract GLDToken is ERC777 {
    constructor(uint256 initialSupply, address[] memory defaultOperators)
        ERC777("Gold", "GLD", defaultOperators)
    {
        _mint(msg.sender, initialSupply, "", "");
    }
}

For #1, I compiled on Remix and tried to deploy via Remix VM (Merge)
Tried Solidity versions 0.8.13 - 0.8.19
Tried with 0 optimization and at 1,000,000
Error:

status | false Transaction mined but execution failed

My input:

{
    "uint256 initialSupply": "1234",
    "address[] defaultOperators": [
        "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4"
    ]
}

It seems it reverts on line 72 on the ERC777.sol file

_ERC1820_REGISTRY.setInterfaceImplementer(address(this), keccak256("ERC777Token"), address(this));

For #2, I did all of the same things but I got

creation of GLDToken errored: Returned error: {"jsonrpc":"2.0","error":"Invalid transaction envelope type: specified type \"0x2\" but included a gasPrice instead of maxFeePerGas and maxPriorityFeePerGas","id":7647309266267112}

For #3, I created a migration file that consisted of

const Implementation = artifacts.require("Implementation");

module.exports = function (deployer) {
    deployer.deploy(Implementation, 1234, ["0x92DF544228Ca92b8C9943BBeECFDA7D3377f6294"]);
};

but when I ran truffle migrate I got

"GLDToken" hit a require or revert statement somewhere in its constructor. Try:
   * Verifying that your constructor params satisfy all require conditions.
   * Adding reason strings to your require statements.

    at /usr/local/lib/node_modules/truffle/build/webpack:/packages/deployer/src/deployment.js:330:1

I am running Truffle v5.8.1 (core: 5.8.1) and Node v18.8.0 and on MacOS Monterey.

Does anyone know what is going on? Or how to fix any of these?

@Amxx
Copy link
Collaborator

Amxx commented Mar 20, 2023

Hello @chriscrutt

Has you figured out yourself, ERC777 requires the ERC1820 registry to be deployed at 0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24. This is the case on many chains.

Unfortunatelly, that is not the case of local testnets such as ganache or remix.

If you use hardhat, @openzeppelin/test-helpers provides toolkit to deploy the ERC820 registry. Otherwize, you can do that manually by:

This is one of the difficulties associated with ERC-777 that lead us to deprecate it.

@chriscrutt
Copy link
Author

chriscrutt commented Mar 20, 2023

@Amxx

This is one of the difficulties associated with ERC-777 that lead us to deprecate it.

That's a bummer- I really liked the built-in functionality to have my smart contract do something when receiving tokens- or forcing a smart contract to react to receiving my tokens haha. Is that why Truffle is failing as well? And that its error statement is just a little too vague.

edit: I looked into #2620 and I suppose I will NOT be missing ERC777 haha.

@chriscrutt
Copy link
Author

what would you recommend instead of ERC777 to get the same effect?

@Amxx
Copy link
Collaborator

Amxx commented Mar 20, 2023

I would recommand a combination or ERC20 and ERC1363.

@chriscrutt
Copy link
Author

Yeah I just saw that OZ was thinking of adding 1363- I know they have the interface for it. Already started to implement.

I have trouble figuring out why 777 is so bad compared to 1363 though

@Amxx
Copy link
Collaborator

Amxx commented Mar 20, 2023

ERC-777 relies on ERC-1820 a lot, that makes it is a painfull point.

I was saying ERC-20+1363 because I assumed you needed "callbacks". If you don't, then 20 alone is good.

@chriscrutt
Copy link
Author

So if I have a staking contract and want something to happen whenever someone transfers the ERC1363 to my smart contract, it'd be super easy with transferAndCall. The only thing I'm worried about is if they just do the transfer function that doesn't have the callbacks integrated.

I would just override transfer so my contract will always do something upon receiving the ERC1363? But transferAndCall requires the ERC1363 to be sent to a contract address so transfer would be unusable to send p2p unless I put in another if (!recipient.isContract()) check and handled the transfer accordingly.

Or is there a better way to go about it?

@Amxx
Copy link
Collaborator

Amxx commented Mar 22, 2023

When using ERC20 there is indeed the risk of someone doing a direct transfer when they should not. That is an intrisic issue with ERC20, but still you should not change the transfer behavior (you risk breaking composability with apps like uniswap).

There is an ongoing issue with ERC1363 reverting (or not) when doing transferAndCall to an EOA. This issue is the reason why we don't yet have an official ERC1363 implementation in the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants