feat: MsgToL1 hash computation

[glihm]

Related to #448.

As suggested in the issue, this is the responsibility of L1HandlerTransaction to expose it's message that can be parsed from it's content.

The use of EthAddress solve the issue concern about a proper type to handle the from_address field.

Also, this PR includes the computation of MsgToL1, which is very practical in some scenarios when testing auto-consume of messages on L1.

[glihm]

@xJonathanLEI could you try to re-run the action? Or could I do that on my end without adding a new commit?

[xJonathanLEI]

Rebased and squashed PR.

[xJonathanLEI]

I don't like how this impl uses BigUint to represent a Keccak hash. Keccak hashes are always 256 bits. You shouldn't need to resort to allocating for this.

[xJonathanLEI]

Also this impl uses the hasher inefficiently. You shouldn't need to build a buffer to feed the hasher at the end. The hasher takes slices.

[xJonathanLEI]

This assert! is completely unacceptable. You're assuming that whatever self instances always come from a trusted source, which is not true. As a library, you cannot decide this for your users. Only invariants can be checked with panic in a library.

[glihm]

got it, thank you for pointing this out.

[xJonathanLEI]

The MsgToL2 part has been superseded by #495. For the MsgToL1 part though, I'm unable to confirm whether the to_address field should be padded to 32 bytes or not.

Based on the official docs, it's not padded (no wrapping wiith uint256() on ToAddress), unlike the L1->L2 counterpart. However, Starkscan shows hashes that are apparently computed with the 32-byte padding.

I'm holding off adding the L2->L1 hashing until this is resolved. Maybe @amanusk can shed some light on this.

[glihm]

The MsgToL2 part has been superseded by #495. For the MsgToL1 part though, I'm unable to confirm whether the to_address field should be padded to 32 bytes or not.

Based on the official docs, it's not padded (no wrapping wiith uint256() on ToAddress), unlike the L1->L2 counterpart. However, Starkscan shows hashes that are apparently computed with the 32-byte padding.

I'm holding off adding the L2->L1 hashing until this is resolved. Maybe @amanusk can shed some light on this.

If we take the code from Starknet Core Contract, the ToAddress must be padded as it's casted in u256 here. Maybe a missing detail in the documentation that can help having it clear without reading the L1 code.

    function consumeMessageFromL2(uint256 fromAddress, uint256[] calldata payload)
        external
        override
        returns (bytes32)
    {
        bytes32 msgHash = keccak256(
            abi.encodePacked(fromAddress, uint256(msg.sender), payload.length, payload)
        );

        require(l2ToL1Messages()[msgHash] > 0, "INVALID_MESSAGE_TO_CONSUME");
        emit ConsumedMessageToL1(fromAddress, msg.sender, payload);
        l2ToL1Messages()[msgHash] -= 1;
        return msgHash;
    }

[glihm]

I don't like how this impl uses BigUint to represent a Keccak hash. Keccak hashes are always 256 bits. You shouldn't need to resort to allocating for this.

It was a suggestion to perhaps have user not bothering for conversion. But as you mentioned, as a library, being as agnostic as we can is far better.

[xJonathanLEI]

Please rebase instead of merging.

[xJonathanLEI]

It was a suggestion to perhaps have user not bothering for conversion.

How do you know the user wants BigUint? Not to mention that BigUint requires an allocation.

[xJonathanLEI]

LGTM