Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EEI: Change address type from integer to bytes #105

Merged
merged 1 commit into from
Oct 7, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eth_interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ The Ethereum Environment Interface exposes the core Ethereum API to the eWASM en

We define the following Ethereum data types:
- `bytes`: an array of bytes with unrestricted length
- `address`: a 160 bit number, represented as a 20 bytes long little endian unsigned integer in memory
- `address`: an array of 20 bytes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Practically this means it is "big endian"? In evmc it still says big endian.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it means big endian, but I usually have trouble reasoning about 160-bit numbers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is currently a case where a 160-bit type should be treated as a value.
The Solidity documentation states that only comparison operators are defined for the "address" type. This puts the type in a difficult situation where it is mostly not a quantity, but can be compared like an integer using operators like <, >, <=, and >=.
I think that the type should not have any notion of byte ordering other than defined by the hash function used to generate it. A correct address value should be the last 20 bytes of the hash of the public key, as usual.

Perhaps this reflects on a needed change in EVMC?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solidity actually considers them numbers (as they derive off IntegerType).

EVM considers them numbers too, since they are "right aligned" in the stack slot.

- `u128`: a 128 bit number, represented as a 16 bytes long little endian unsigned integer in memory
- `u256`: a 256 bit number, represented as a 32 bytes long little endian unsigned integer in memory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we actually consider u128/u256 numbers or just an array of bytes?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I think that would not be sufficient. You would have to explain where these bytes come from, i.e. how numbers where serialized to bytes, i.e. endianness.

In case of address this is not needed because an address is a well specified part of a hash, where a hash is defined as bytes, not a number.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u128 should be defined as an integer value, since it (usually?) denotes an ether value and native arithmetic and comparison operators should work on them in the WASM environment (by experimental u128 or otherwise).
u256 is very difficult because a hash value should not be byte-swapped but the difficulty should be, as it is a quantity.
Byte ordering is therefore highly dependent on context in the case of u256.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So make two types.


Expand Down