ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.87k stars 5.27k forks source link

Questions (and suggestions) for EIP1577 #1663

Closed angerborn closed 5 years ago

angerborn commented 5 years ago

Hey, I have a few questions about EIP1577.

I think you'd be able to answer these questions quickly @Arachnid @decanus .

Is it correct to call the \\ a multicodec?

Over at multiformats/multicodec it's specifically stated that:

we will keep a globally agreed upon table with common protocols and formats.

Since EIP1577 links a special version of the table here, I think it could be a bit confusing or misleading to call this a multicodec. Anyone implementing this for example will not be able to use any of the implementations linked from the multicodecs github.

Also, what's the difference between address and codec in this case and why did the eip switch from using address to codec?

What about types outside the 0x00-0xf0 range?

The encoding of the value depends on the content type specified by the protoCode; for instance, types in the range 0x00-0xf0 are encoded using multihash

Have I missed the definition for encoding above these values, are they defined elsewhere or not at all? Is it relevant to consider these values at all?

Suggestion: Link or describe which varint implementation is intended

This is the one multiformats use, I'm assuming it is the one? There are the bit-encoded versions such as this one, but there also seems to exist versions that use one byte to specify the size. Since there are more than one version, it'd be nice to directly link to it somewhere in the EIP.

Suggestion: Extend the fallback time

It's specified that fallback must only be supported until Dec 31st 2018, which isn't very far away now. Since the eip is still a draft, perhaps it's decent to actually keep the fallback for a while even after it's final?

angerborn commented 5 years ago

I'm closing this as apparently there are planned changes to the EIP currently.

https://github.com/MetaMask/metamask-extension/issues/5742#issuecomment-449456468