i believe this will save some gas, vs. computing in the constant (which I think turns it into a function)

according to https://emn178.github.io/online-tools/keccak_256.html it is
0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9

this information would've probably been more useful to you earlier, but I've just heard about your project. so, WETH was created with Kelvin versioning.

dapphub/ds-weth@becce32

https://chat.makerdao.com/channel/oasis?msg=sy73LpH8EYLdXDBX2

if you're not sure what Kelvin versioning is, here's an explainer: https://jtobin.io/kelvin-versioning

while the augmentations you added are very interesting, this should be rather called WETH8. calling it WETH10 is ignoring the spirit in which WETH was created and what its developers stood for.

tough there may be a reason why your implementation is not WETH8, which I'll explain in the next issue.

To prevent expensive mistakes, the WETH contract should prevent transfers to the token contract.

I've put together two different implementations of this:

#38 adds a few require checks to prevent self transfers
#39 uses the existing overflow protection to prevent self-transfers without incurring any additional gas costs on each transfer

It is not possible to distinguish Transfer(wallet, 0, amount) event happened because of the wrong transfer or withdrawal operation.

There are so many goodies included, but how do you tell that you actually support them?

Look no further: https://eips.ethereum.org/EIPS/eip-165

Tell other contracts what you have on menu.

Bring this from twitter : https://twitter.com/wighawag/status/1319658306697371649

As currently implemented, the function totalSupply is not compliant with the ERC20 standard if the following happen:

• ETH is sent to WETH10 through a self destruct
• during a flash loan

For totalSupply to be compliant,

1. the sum of all Transfer (from zero address) minus the sum of all Transfer (to zero address) need to be equal to totalSupply
2. the sum of all balances should be equal to totalSupply()

flash loan as currently implemented breaks 2.
self destruct with ETH sent to WETH10 breaks 1.

Lines 45, 74 and 76.

I would consider allowing flash minting of weth.

Per Mariano Conti recommendation: https://twitter.com/nanexcool/status/1314650342236344325?s=21

The ERC-2612 spec defines a public view function for retrieving the permit DOMAIN_SEPARATOR:

function DOMAIN_SEPARATOR() external view returns (bytes32)

This function is missing from WETH10.

Furthermore, the domain separator is currently calculated every time permit() is called. The function would be more gas efficient if the value was calculated once in the constructor and stored.

Instead of calling the msg sender, mint to the given to address and call it

hi, for WETH10 to be maximally useful, it should be deployed on all testnets, ideally at the same address.

Check here: https://etherscan.io/address/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2#code

https://twitter.com/androolloyd/status/1314777925414252544?s=21

This line would not be necessary if instead of the overflow trick you would simply do require(to != address(this), "SELF") which consume less gas
or am I missing something?

Per Mariano Conti recommendation: https://twitter.com/nanexcool/status/1314649151515299840?s=21

Agustin Aguilar analysis of gas savings: https://twitter.com/agusx1211/status/1313759898258362378?s=21

This file would be useful for anyone who wishes to integrate with WETH10

I feel really bad that we rely on actual contract balance here: https://github.com/WETH10/WETH10/blob/main/contracts/WETH10.sol#L56

/// @dev Returns the total supply of WETH10 as the Ether held in this contract.
function totalSupply() external view override returns(uint256) {
  return address(this).balance + flashSupply;
}

We should understand that balance could be increased externally and would not match the sum of balances plus flashSupply:

• Remember that Ether can be forcibly sent to an account
• Forcibly Sending Ether to a Contract

if ethereum forks and a new chain id is assigned, the domain separator will be outdated.

either allow updating the domain separator or compute it every time

the risk here is replay attacks on both chains

test the gas efficiency of every function

Assuming these numbers:
300000 daily transactions.
40 Gwei/GAS
400 USD/ETH

Each require to stop transfers to the WETH contract is ~30 gas. That's about $200 per day.

There are $21000 locked in the WETH contract after three years of operation. The require will waste that much in less than four months.

including moving away all test suites from Truffle/OZ env

@albertocuestacanada what do you think Alberto, is this something that you wish for?

imo not really any more usable than deposit and not gas cheaper

Another no brainer in the wETH upgrade contract would be a way to take advantage of CREATE2 method to allow users to associate an address with 'wrapping ether' regardless of chainID.

name, symbol, decimals, DOMAIN_SEPARATOR and PERMIT_TYPEHASH

Having chainId (and as a result the domain separator) set in the constructor will result in issues if a hardfork happen for the chain that will need to change the chainId :

When a contract is self-destructed, the remaining ETH is sent to a designated address.

Afaik, this is a way to send ETH to a contract that doesn't trigger that contract's receive function (formerly the fallback function).

Could this cause WETH10 to malfunction?

There is no reason to have those messages as terse as they are (e.g. !recipient)

It reduces contract size but this is a one time deployment

I will admit, I am not entirely versed on this issue, but tagging @Agusx1211 per recommendations here.

The Readme says:

Total Supply
The supply of WETH10 is capped at type(uint112).max.

However, the code defines it as:

As flashMinted alone can reach type(uint112).max:

It seems that address(this).balance + flashMinted can exceed type(uint112).max.

USDCv2 has option to recover funds

USDC proxy: https://etherscan.io/token/0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48#readProxyContract

Implementation: https://etherscan.io/address/0xb7277a6e95992041568d9391d09d0122023778a2#code

Starts at line 1313:

contract Rescuable is Ownable {
    using SafeERC20 for IERC20;

    address private _rescuer;

    event RescuerChanged(address indexed newRescuer);

    /**
     * @notice Returns current rescuer
     * @return Rescuer's address
     */
    function rescuer() external view returns (address) {
        return _rescuer;
    }

    /**
     * @notice Revert if called by any account other than the rescuer.
     */
    modifier onlyRescuer() {
        require(msg.sender == _rescuer, "Rescuable: caller is not the rescuer");
        _;
    }

    /**
     * @notice Rescue ERC20 tokens locked up in this contract.
     * @param tokenContract ERC20 token contract address
     * @param to        Recipient address
     * @param amount    Amount to withdraw
     */
    function rescueERC20(
        IERC20 tokenContract,
        address to,
        uint256 amount
    ) external onlyRescuer {
        tokenContract.safeTransfer(to, amount);
    }

    /**
     * @notice Assign the rescuer role to a given address.
     * @param newRescuer New rescuer's address
     */
    function updateRescuer(address newRescuer) external onlyOwner {
        require(
            newRescuer != address(0),
            "Rescuable: new rescuer is the zero address"
        );
        _rescuer = newRescuer;
        emit RescuerChanged(newRescuer);
    }
}

Per Mariano Conti recommendation: https://twitter.com/nanexcool/status/1314651709336150017?s=21

The WETH10 implementation contains dead code:

This code can never execute successfully as to == address(0) so that the Solidity-integrated check will fail for the following line:

Dead code in principle is not bad, it just fails later than it should and could confuse integrators.

Having weth9 in the constructor is unnecessary and prevent the use of create 2 for deterministic address across chains (see #22 (comment)) unless weth9 has itself the same address across chains (but believe it is not the case, hence :

Instead of having WETH10 tied to WETH9 the various convert functions could accept an extra argument specifying the WETH contract that should receive the eth.

This would also allow easy transition to a future WETH11 if any.

Just copy this and this.

Just before deployment to mainnet, make all functions external, allowing code duplication. This will cut on gas costs.

Before the contract is ready for mainnet deployment, leave as is.

Who do we think might step up here? wrapping ether into tokenized form in "best o' class solidity" feels like no brainer common good.

ethereum/EIPs#1271

This would allow contract signatures to be used for WETH10

the ternary is not worth it when 99% of the time balanceOf isn't called on the contract, and you should just have an invariant test that checks the token balance of the contract is always 0

related is removing the function altogether and just renaming _balanceOf to balanceOf

This is not acceptable.

-------------|----------|----------|----------|----------|----------------|
File         |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
-------------|----------|----------|----------|----------|----------------|
 contracts/  |       64 |    32.14 |    66.67 |    62.26 |                |
  WETH10.sol |       64 |    32.14 |    66.67 |    62.26 |... 6,78,82,117 |
-------------|----------|----------|----------|----------|----------------|
All files    |       64 |    32.14 |    66.67 |    62.26 |                |
-------------|----------|----------|----------|----------|----------------|

I'm not sure with the permit test fails when running coverage.

The goal is to have 100% coverage of statements, but let's start with the functions.

https://github.com/WETH10/WETH10/blob/main/contracts/WETH10.sol#L12

This interface isn't necessary, because the msg sender already knows the amount they called flash mint with, so just call with the data

This is not really a technical vulnerability, but the current implementation leaves the door open for economic manipulation.

The current flashMint function doesn't enforce a cap on how much money there can be minted, which means that anybody can get as much as uint256(-1) WETH. It's true that flash loans are truly useful only when transferred to another contract to perform some sort of trade, and attempting to transfer uint256(-1) tokens would revert. But what if there's an external system that can be influenced solely by the return value of the balanceOf function in WETH10?

Suppose that a third-party community needs to settle a contentious debate via a poll and for whatever reason they decide to use WETH10 balances to tally votes. Further suppose that they're not taking a snapshot of the blockchain, but they are using an on-chain implementation that checks for the balances in the WETH10 contract. Ethereum did something similar in 2016 with Carbonvote.

A shrewd attacker would wait until the voting period is almost done, flash mint a bajillion WETH10 and ballot their vote, influencing the result in their favour.

I know that exactly because of the reasons expounded in this post nobody will want to use WETH10 (in the current form) for vote tallying, or if they really wanted to, they would take a snapshot of the blockchain, or they would re-do the vote - but why not eliminate the possibility altogether?

Let's add a check that the updated balance of the caller is not bigger than the total supply:

function flashMint(uint256 value, bytes calldata data) external lock {
    balanceOf[msg.sender] += value;
    require(balanceOf[msg.sender] >= value, "overflow");
    // Add this check
    require(balanceOf[msg.sender] <= totalSupply(), "!totalSupply");
   ...
}

Especially when calculating keccak256:

EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)

and

Permit(address src,address guy,uint256 wad,uint256 nonce,uint256 deadline)

Any thoughts about a function that would allow easy, cheap conversion from WETH9 into WETH10? Maybe reverse too?

Any thoughts on including transferAndCall? Should hopefully make some transactions cheaper by not needing to use allowances.

ethereum/EIPs#677

Bringing over a discussion from Twitter.

I think that the current way the flash mint function is built is dangerous because it breaks the invariant that total supply of a token should never exceed 2^256 -1 (or MAX_UINT256).

https://github.com/WETH10/WETH10/blob/main/contracts/WETH10.sol#L111-L125

Giving a specific example of how this invariant could be broken:

total supply == sum(account balances)

balance(account_1) == 100 Wei

**Account 2 uses Flash Mint**

balance(account_2) == 2^256 - 1 Wei

total supply == 2^256 + 99 Wei > 2^256 - 1 Wei

Even though this invariant is not very clearly specified in the ERC20 standard, I do believe it is widely accepted and implicitly used by many of Ethereum's developers.
And so, I firmly believe that, if the flash minting functionality is deployed to main net, it should also abide to this invariant.

ERC677 is not well documented (for example the ERC currently mentioned emitting special Transfer event, which is a really bad idea) and transferAndCall require special support from receiver

Here are some discussion on alternatives : https://ethereum-magicians.org/t/about-677-and-other-functions-on-top-of-erc20/4589

I locked the eth functions while flash minting as a precaution, but it's that needed? Should we just assert that the eth balance is equal or greater than the weth supply at the end of a flash mint?

The lock uses 5K gas, so removing it would be great.

On the other hand, could anything be broken with a billion dollars worth of ETH?