huff-language/huffmate

A library of modern, hyper-optimized, and extensible Huff contracts with extensive testing and documentation built by Huff maintainers.

huffmate's Issues

feat: Metamorphic Contracts and Factory

Overview

More of a fun one!
Implement 0age's Metamorphic contract factory.

`testOnlyContract` is failing

https://github.com/pentagonxyz/huffmate/blob/6798bcc97026e69ff0c4bb3e0c914d2bbcb8ba7c/test/auth/OnlyContract.t.sol

    /// @notice Tests that ONLY_CONTRACT macro reverts when tx.origin == msg.sender
    function testOnlyContract(address caller) public {
        vm.assume(caller != address(this));

        // Should revert when tx.origin == msg.sender
        // vm.startPrank(address,address) sets msg.sender and tx.origin
        vm.startPrank(caller, caller);
        vm.expectRevert();
        only_contract.call("");
        vm.stopPrank();

        // Only setting the msg.sender and not tx.origin should succeed, so long as caller != address(this)
        vm.startPrank(caller);
        (bool success,) = only_contract.call("");
        assert(success);
        vm.stopPrank();
    }

The default tx.origin of forge is 0x00a329c0648769A73afAc7F9381E08FB43dBEA72 which is different from address(this). So when the fuzzing tries with caller = 0x00a329c0648769A73afAc7F9381E08FB43dBEA72, the test fails.

Opti: remove double iszero before jumpi

Overview

There are several contracts where there are two iszero in a row before a jumpi to conditionally jump if a value is different from 0. But those are not necessary.
From evm.codes:

b: the program counter will be altered with the new value only if this value is different from 0. Otherwise, the program counter is simply incremented and the next instruction will be executed.

For example, in case of a call (delegate, static, or classic), they return success:

success: return 0 if the sub context reverted, 1 otherwise.

So, if there is a jumpi just after a call, there is no need to do iszero iszero.
Using the success directly from the stack is sufficient.
Removing those double iszero could save 2 instructions each times those macros are called.

Todo

Contracts where there are possible improvements:

• ERC4626 (see PR)
• ERC1967Upgrade (see PR)
• ERC20_TRANSFER (utils)
• SSTORE2_WRITE (after a create but same principle applies)
• ERC721 (check if an address is != address(0))

Separate mock/wrapper macros from methods

atm some of the contracts contain the mock logic used for testing directly in the contract. this decreases readability and we should instead use a consistent pattern for separating the mocks from the actual methods. the pattern used here is imo ideal: https://github.com/pentagonxyz/huff-lib/blob/main/src/data-structures/mocks/InstantiatedHashmap.huff

failing tests

Please, what would be the path to fix the failing tests ?

getting: Test result: FAILED. 16 passed; 30 failed; 0 skipped; finished in 5.43s

huffc:  0.3.2
foundry: nightly-4a643801d0b3855934cdec778e33e79c79971783

include ERC72.huff got duplicate macro name

I got the error of compiling that it told me I have duplicate macro function, when I am writing the huffs with using huffmate, I think the reason is ERC721 included two files that are CommonErros.huff and nonpayable.huff, they all imported the same file that is Errors.huff. My #include visions are following:

MyTest.Huff ---> #include ECR721.huff

or

MyTest.Huff ----> #include hashmap.huff & #include ECR721.huff

above situations can cause the errors that including of duplicate to happen.

feat: Create3

Overview

Create a huff equivalent of Solmate's Create3.

issue: ERC20 decimals should be uint8

According to ERC20 especification the decimals shoud be defined as;

function decimals() public view returns (uint8)

But in ERC20.huff#L29 this is the definition;

#define function decimals() nonpayable returns (uint256)

Shouldnt be this definition?

#define function decimals() nonpayable returns (uint8)

feat(v2): Consensus Algos

Overview

Port over the beacon chain consensus helper algos to huffmate (in their respective folders)

Follow huff styIe guide ?

It seems that jtriley-eth's style guide (https://github.com/jtriley-eth/huff-style-guide) is not followed across the whole codebase. Is it something that should be fixed ?

Originally posted by @MathisGD in #129 (comment)

feat: Bytes

Overview

Implement the Bytes.huff contract located in src/data-structures/Bytes.huff.

The Bytes.huff contract should contain helpers for working with Bytes in Huff, similar to Arrays.huff in src/data-structures/Arrays.huff.

fix: Rip out Mechanisms

Overview

huffmate's goal is hyper-optimized utility contracts that can be built ontop of.

mechanisms do not fit this goal and ought to be ripped out into separate repositories.

feat: ProxyFactory + Proxy

Overview

We need a factory for creating proxies similar to openzeppelin's Clones.sol. Will also require the introduction of a Proxy.sol contract.

Shouldnt the ERC721 implementation revert on `balanceOf(address(0))`?

This is a quote from eip-721

/// @notice Count all NFTs assigned to an owner
/// @dev NFTs assigned to the zero address are considered invalid, and this
/// function throws for queries about the zero address.
/// @param _owner An address for whom to query the balance
/// @return The number of NFTs owned by _owner, possibly zero
function balanceOf(address _owner) external view returns (uint256);

I dig into other implementations and see that balanceOf(address(0)) reverts;

• OZ ERC721.sol#L63
• Solmate ERC721.sol#L40
• Solady ERC721.sol#L145

But in the Huffmate implementation is returning always 0, shoulndt revert instead?
Please view ERC721.t.sol#L461-L464

Buttplug

fix: Dynamic Constructor Args

Overview

Tokens currently don't use dynamic constructor arguments, but rather static strs.

optimize: SafeTransferLib

Overview

At the moment, the SafeTransferLib implementation uses right padded function selectors (see: https://github.com/pentagonxyz/huffmate/blob/main/src/utils/SafeTransferLib.huff#L25). Instead, we should follow the original Solady method of storing a non-padded selector and using mem[28:..] for the calldata (example) to reduce codesize.

feat: ECDSA Lib

Overview

Port Solady's ECDSA.sol

feat: Art Gobblers

Overview

Port the Art Gobblers contracts to huffmate :catjam:

feat(v2): Safe Operations with Low Level Integers

Description

Pending #103, implementations for safely operating on low level integer types in huff can be done.

As detailed by @AmadiMichael:

Example: an extra macro in the safeMath library called SAFE_ADD_TYPE(), the same as SAFE_ADD() but would take a third stack input which is the max value of the intended type to cast the result into

/// a rewrite of safe add with max type

/// @notice Adds two numbers and reverts on overflow
#define macro SAFE_ADD_TYPE() = takes (3) returns (1) {
    // input stack          // [num1, num2, typeMax]
    dup2                    // [num2, num1, num2, typeMax]
    add                     // [result, num2, typeMax]
    dup1                    // [result, result, num2, typeMax]
    swap2                 // [num2, result, result, typeMax]
    gt                         // [is_overflow, result, typeMax]
    iszero                  // [is_not_overflow, result, typeMax]

    is_not_overflow jumpi   // [result, typeMax]
        [ARITHMETIC_OVERFLOW] PANIC()

    is_not_overflow:        // [result, typeMax]
        swap1              // [typeMax, result]
        dup2                // [result, typeMax, result]
        gt                     // [is_type_overflow, result]
        iszero              // [is_not_type_overflow, result]

        is_not_type_overflow jumpi   // [result]
         [ARITHMETIC_OVERFLOW] PANIC()

    is_not_type_overflow:
}

This can help in safe calculation of values of lower types like uint128 etc. After the initial overflow check (which accounts for only 256 bits) there's a check to ensure it doesn't overflow from the type you intend to keep it within.

someone can call this with

#include "utils/Constants.huff"

...

#define macro ADD_UINT128() = takes(0) returns(1) {
       [__UINT128_MAX]           // [Uint128Max]
       0x01                                // [0x01, uint128Max]
       0x02                               // [0x02, 0x01, uint128Max]
       SAFE_ADD_TYPE()                  // [result]
}

Returns a result in uint128

This will revert if trying to add numbers where either of them is higher than the max of result's type.

Duplicated `APPROVE` macro in the ERC20 contract

This is a good find from sashik.eth, so reposting here for tracking purposes, and it's a "good first issue" for anyone who wanna take it.

"
Why in huffmate/ERC20 "approve" macro declared twice?
https://github.com/huff-language/huffmate/blob/main/src/tokens/ERC20.huff#L98
https://github.com/huff-language/huffmate/blob/main/src/tokens/ERC20.huff#L248
"

By the way, one message later, he mentioned that huff-rs should not allow this to compile. So the current huffmate ERC20 is broken

"
Previous versions of huff allowed to compile this? because with the current I receive:
Error: Duplicate MACRO name found: \"APPROVE\" \nMACRO names should be unique
"

Adding custom errors

Currently there is the REQUIRE that lets you mimic solidity require(condition, "message")

I want to propose a REQUIRE_CUSTOM_ERROR:

#define macro REQUIRE_CUSTOM_ERROR() = takes(2) returns(0) {
    // [condition, customErrorSig]
    do_not_throwErr jumpi
    0x00 mstore
    0x04 0x00 revert

    do_not_throwErr:
    pop
}

feat: REVERT Macro

Overview

Currently, every function selector match requires a redundant revert (eg 0x00 0x00 revert) when the function selectors don't match in the main macro.

If huff-rs gets support for a syntactic sugar string literal -> hex conversion, we could create a new Revert.huff utility that contains a Revert macro to make this redundancy simpler by just calling the macro like: REVERT() after no function selectors match.

Demonstration

#define function getCaller() view returns (uint256)

#define macro MAIN() = takes (0) returns (0) {
    pc calldataload 0xE0 shr
    dup1 __FUNC_SIG(getCaller) eq getCaller jumpi

    // No function selector matches, use new REVERT macro
    REVERT()

    getCaller:
        caller 0x00 mstore
        0x20 0x00 return
}

feat: Refunded

Overview

Port Zolidity's Refunded.sol to huff.

fix: `SafeTransferLib` tests

Overview

In SafeTransferLib's tests, we don't currently exclude the hardhat console or vm addresses from the non-contract tests. As pointed out in #119, this can sometimes cause the fuzz tests to fail.

Solution

Exclude the hardhat console or vm addresses from the non-contract tests. Bonus points for replacing the parameter restrictions with vm.assume so we get full advantage of all fuzz runs.

bug: SafeTransferLib

This PR uncover a bug in SafeTransferLib by fixing the fuzzing.

This needs investigation.

Shouldnt the ERC721 implementation revert on `ownerOf(address(0))`?

This is a quote from eip-721

/// @notice Find the owner of an NFT
/// @param _tokenId The identifier for an NFT
/// @dev NFTs assigned to zero addresses are considered invalid, and queries
/// about them do throw.
/// @return The address of the owner of the NFT
function ownerOf(uint256 _tokenId) external view returns (address);

I dig into other implementations and see that ownerOf(address(0)) reverts;

• OZ ERC721.sol#L72
• Solmate ERC721.sol#L36
• Solady ERC721.sol#L130-L134

But in the Huffmate implementation is returning always 0, shoulndt revert instead?
Please view ERC721.t.sol#L821-L824

Buttplug