chore: remove useless `and` op

[zhiqiangxu]

sload always returns 32 bytes, so no need to truncate.

[github-actions[bot]]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[zhiqiangxu]

I have read the CLA Document and I hereby sign the CLA

[nlordell]

I believe this truncates the loaded 32-byte word to the least significant 20-bytes, to ensure it is a well-formed address.

[zhiqiangxu]

Oops, I miscounted the bits, thanks for the clarification!

[nlordell]

To follow up on this @zhiqiangxu - it looks like the and may be useless afterall.

Looking at the Ethereum yellow paper, which specifies the EVM semantics:

It looks like the masking is done implicitly by the EVM, and does not need to be done manually by the code.

That being said, the masterCopy() function handling still needs to mask the address in order to remain Solidity ABI compatible (but this is an edge case).

Credit goes to @mmv08 for finding this!

[coveralls]

Pull Request Test Coverage Report for Build 7843219952

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 94.397%

---

Totals
Change from base Build 7829762295:	0.0%
Covered Lines:	396
Relevant Lines:	404

---

💛 - Coveralls

[zhiqiangxu]

That's a good finding, I can also confirm that the up 12 bits don't matter from the geth implementation.

[mmv08]

There's also this section in the yellow paper confirming this:

[zhiqiangxu]

I added the masking back for masterCopy(), maybe this is also unnecessary since the upper 12 bits should also be zeroed when loaded from slot 0(if the solc doesn't try to make use of the upper 12 bytes for this slot), but I don't have an evidence from the solidity spec.

[zhiqiangxu]

before: SafeProxy 171 bytes (limit is 24576)

after applying mstore(0, shr(12, shl(12, _singleton))): SafeProxy 155 bytes (limit is 24576)

after applying mstore(12, shl(12, _singleton)): (This assumes the memory 0-12 is guaranteed to be 0'ed out. ) SafeProxy 152 bytes (limit is 24576)

after applying mstore(0, _singleton) (This assumes the upper 12 bytes are always zeroed out after loaded from slot 0) SafeProxy 149 bytes (limit is 24576)

The last commit applies mstore(0, shr(12, shl(12, _singleton))).

[nlordell]

I added the masking back for masterCopy(), maybe this is also unnecessary since the upper 12 bits should also be zeroed when loaded from slot 0 (if the solc doesn't try to make use of the upper 12 bytes for this slot), but I don't have an evidence from the solidity spec.

I don't think this is the case. In particular, it is the EVM that specifically doesn't make use of the upper 12 bits, and not solc. If they are set in storage, then masterCopy() would not return bytes that follow the Solidity ABI spec. In particular from the Solidity documentation:

• uint<M>: enc(X) is the big-endian encoding of X, padded on the higher-order (left) side with zero-bytes such that the length is 32 bytes.
• address: as in the uint160 case

Which requires the returned bytes to be 0-padded.

So, in order for the proxy to work with any value stored in the storage slot 0, then we need to mask for the masterCopy call. In practice, the 12 leading bytes of slot 0 will always be 0, however, we cannot guarantee that because of DELEGATECALL shenanigans.

after applying mstore(12, shl(12, _singleton)): (This assumes the memory 0-12 is guaranteed to be 0'ed out. ) SafeProxy 152 bytes (limit is 24576)

I think in practice for this contract it is always 0, but I think it is slightly wrong to rely on undefined behaviour here which may change in future Solidity compiler versions.

The last commit applies mstore(0, shr(12, shl(12, _singleton))).

Personally, I think this is the way to go.