filecoin-project / filecoin-solidity

Filecoin Solidity API Library
Other
17 stars 11 forks source link

Implementation of toEthAddress function #23

Closed vaniiiii closed 10 months ago

vaniiiii commented 10 months ago

Description

This pull request adds the required functionality related to issue #4

Developers can utilize this function to convert a f4 FileAddress type into an EthAddress.

Approach and implementation

The starting point was PR #16 which already included the implementation of this function. However, unit tests for various types of addresses were missing. The missing unit tests are added while retaining the existing fuzz tests with minor changes.

Validate function changes

The validate function checks the FilAddress in bytes format, which includes a protocol byte and payload.

The previous implementation was failing for valid input because it was comparing the bytes length with 10:

if (addr.data[0] == 0x00) {
    return addr.data.length <= 10; // should be <= 11 
}

This approach is incorrect, as the f0 format contains a protocol byte and payload, which can be a maximum of 10 bytes long. An example is provided in the Filecoin spec document, in the last example in section 6.1.3.1.

|----------|---------------|
| protocol |    payload    |
|----------|---------------|
|    0     | leb128-varint |

Discussion

Affected issues

maciejwitowski commented 10 months ago

Should the validate function also include checks for minimum length for f0 and f4 addresses? For example, should 0x00 be considered a valid f0 address?

It shouldn't. FIP-54 says that F0 converted to Masked address must be 20 bytes long.

|- disc -|    |----- padding ------|    |----- actor id -----|
0xff       || 0000000000000000000000 || <uint64 ID big endian>

Also, feel free to check the code related to id addresses and ETH addresses in fevmate library.

maciejwitowski commented 10 months ago

Should f4 addresses be checked against a length of 65 if the payload is hardcoded to 54, as specified in FIP. If the payload is not 64 - prefix.len(), the total length, according to the format, should be 65 bytes. More details can be found in this https://github.com/filecoin-project/FIPs/discussions/459#discussioncomment-3648766

I'd need @Stebalien's help answering this one.

Stebalien commented 10 months ago

Should f4 addresses be checked against a length of 65 if the payload is hardcoded to 54, as specified in FIP. If the payload is not 64 - prefix.len(), the total length, according to the format, should be 65 bytes. More details can be found in this https://github.com/filecoin-project/FIPs/discussions/459#discussioncomment-3648766

The FIP is discussing the f4 address class which is a superset of the Ethereum address space (f410f...).

Ethereum f4 addresses always:

  1. Start with 0x040a (4 for the f4 address class, 10 for the ID of the EAM).
  2. End with 20 bytes, the Ethereum-style address.

So a 0x (ethereum) address converted to f4 should always be 22 bytes (exactly).

vaniiiii commented 10 months ago

Should f4 addresses be checked against a length of 65 if the payload is hardcoded to 54, as specified in FIP. If the payload is not 64 - prefix.len(), the total length, according to the format, should be 65 bytes. More details can be found in this filecoin-project/FIPs#459 (reply in thread)

The FIP is discussing the f4 address class which is a superset of the Ethereum address space (f410f...).

Ethereum f4 addresses always:

  1. Start with 0x040a (4 for the f4 address class, 10 for the ID of the EAM).
  2. End with 20 bytes, the Ethereum-style address.

So a 0x (ethereum) address converted to f4 should always be 22 bytes (exactly).

Thank you for the clarification, @Stebalien and @maciejwitowski. It makes perfect sense for Ethereum address space. For f4 addresses beyond Ethereum address space, it should be max 64 bytes?

@maciejwitowski, in this particular contract, it's important to note that it only handles the "basic" address format, which consists of a protocol and a payload. The conversion to and from an id and ethAddress is managed by a separate contract, FilAddressIdConverter.sol. Therefore, based on this specific format, the minimum length for f0 should be 2 bytes (1 for the protocol and at least 1 for the actor id), and the maximum length should be 11 bytes.