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

Dynamically sized bytes type. #280

Open
g-r-a-n-t opened this issue Mar 2, 2021 · 7 comments
Open

Dynamically sized bytes type. #280

g-r-a-n-t opened this issue Mar 2, 2021 · 7 comments
Labels
comp: analyzer Everything that involves the analyzer pass comp: compiler
Milestone

Comments

@g-r-a-n-t
Copy link
Member

g-r-a-n-t commented Mar 2, 2021

What is wrong?

Dynamically sized byte arrays are commonly used and can be found in the public interfaces of many contracts. For example, it's used here in the Uniswap core contracts. Since it's so common, we need to deal with it somehow.

See #258 for discussion about the future of bytes in Fe. For now tho, we may just want to add a type similar to stringN for practical reasons. We can always remove it at a future date.

How can it be fixed

The easiest way to support bytes would be to do what we currently do with strings. That is, set a limit on the size of the string, but keep it dynamic otherwise.

@g-r-a-n-t g-r-a-n-t added comp: compiler comp: analyzer Everything that involves the analyzer pass labels Mar 2, 2021
@g-r-a-n-t g-r-a-n-t added this to the Uniswap Demo milestone Mar 2, 2021
@cburgdorf
Copy link
Collaborator

For the cited example, we would then use a type such as bytes1000 where 1000 is an arbitrary picked number that is considered "big enough"? And the reason for the upper bound is basically to maintain stronger decidability in the language, right?

@g-r-a-n-t
Copy link
Member Author

Sorry for the late reply @cburgdorf, missed this.

For the cited example, we would then use a type such as bytes1000 where 1000 is an arbitrary picked number that is considered "big enough"? And the reason for the upper bound is basically to maintain stronger decidability in the language, right?

That, but it's also more easily implemented. Giving a max size keeps us from having to deal with multiple segments of memory.

@cburgdorf
Copy link
Collaborator

Citing from your reply in #277

Without support for dynamically sized bytes, I'm not quite sure how this will be implemented.

Do you suggest that msg.data would be of type bytesN where N is a fixed number that we decide that should be big enough to hold the data?

If yes, then, is there even a limit or is the limit just bound by the max block gas limit?

Giving a max size keeps us from having to deal with multiple segments of memory

So I assume whatever bytesN ends up being we would reserve the entire space in memory for it. This means, even if we don't actually end up using it, we end up making memory more access more expensive simply because we reserved that space already.

Btw, it may be that Vyper doesn't support msg.data for this reason:

@g-r-a-n-t
Copy link
Member Author

g-r-a-n-t commented Mar 16, 2021

Do you suggest that msg.data would be of type bytesN where N is a fixed number that we decide that should be big enough to hold the data?

Basically, yeah. I think this is as good as it gets without getting too deep in the weeds.

If yes, then, is there even a limit or is the limit just bound by the max block gas limit?

We don't want to allocate too much space. It could just be some arbitrary number number like 1024. I imagine this would make due in most cases.


It's worth noting that I really don't like this approach, even if it's only temporary.

Maybe there's a completely different way of handling this. For example, we could add a special type that is used to write bytes from calldata into a fixed size bytes array.

pub def foo(some_bytes: cbytes) -> bytes[100]:
  bar: bytes[100]
  some_bytes.read(100, bar)
  return bar

Just an idea.. The typing is still somewhat weird. msg.data would be of type cbytes.

@g-r-a-n-t
Copy link
Member Author

Of all the times bytes is used in Soldity, I wonder which one of the following is most often the reason:

  1. Programmer doesn't actually know the size of the bytes array at compile time.
  2. Programmer cannot express fixed bytes array with size greater than 32.

@cburgdorf
Copy link
Collaborator

cburgdorf commented Mar 16, 2021

It's worth noting that I really don't like this approach, even if it's only temporary.

Yeah, me neither. Choosing a limit that will fit most use case but not all rubs me the wrong way. Especially when these limits are choosen based on the current network use. What if that dramatically changes in a rollup centric world with higher gas limits and transactions with more calldata become the norm.

For example, we could add a special type that is used to write bytes from calldata into a fixed size bytes array.

This is what we should do, I think! But instead of the read API we can go one step further as Vitalik was suggesting in the Vyper issue.

We could introduce an API slice(offset, length) on cbytes that would let you read arbitrary chunks out of it.

Also, we might get away with not having to pass an array explicitly since we could infer the return type directly from the literals 0and 1. Though, that wouldn't work if non-literals were allowed here.

pub def foo(some_bytes: cbytes) -> bytes[100]:
  return some_bytes.slice(0, 100)  # returns bytes[100] based on the inputs of 0 and 100

We can tweak around with the exact APIs but I like this a lot more for!

@cburgdorf
Copy link
Collaborator

Of all the times bytes is used in Soldity, I wonder which one of the following is most often the reason

Hard to tell for me.

Programmer cannot express fixed bytes array with size greater than 32.

As far as I understand you can have a fixed size byte array larger than 32 in Solidity (such as bytes[40]) but it would end up padded so I guess it would consume 64 bytes whereas using bytes would only consume the actual 40 bytes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: analyzer Everything that involves the analyzer pass comp: compiler
Projects
None yet
Development

No branches or pull requests

2 participants