Consistent order in the parameter structs

[PaulRBerg]

Two-pronged task:

• [ ] Investigate whether the current order in the parameter structs (e.g. CreateWithDurations) saves gas
• [ ] If not, rewrite all parameter structs to follow a consistent order:

I.e. this order:

• Sender
• Recipient
• Total Amount
• Asset
• Cancelable
• Transferable
• Time-related variables
• Broker

[PaulRBerg]

@andreivladbrg you said here that the order of parameters does not matter for the parameter structs, but have you checked that?

[smol-ninja]

If I may answer your answer @PaulRBerg, since struct CreateWithDurations is only used in memory or on the stack (and never storage), the order of parameters does not affect gas usage. Each variable has its own slot.

So ordering does not matter here. BUT EVM still has to perform casting to fit into 32 bytes when we are using uint128 or bool. So if we want to save some gas, we should be using uint256 instead of uint128 and bool (each can save ~18 gas).

[PaulRBerg]

since struct CreateWithDurations is only used in memory or on the stack (and never storage), the order of parameters does not affect gas usage. Each variable has its own slot.

Wonderful, thanks for confirming!

UT EVM still has to perform casting to fit into 32 bytes when we are using uint128 or bool. So if we want to save some gas, we should be using uint256 instead of uint128 and bool (each can save ~18 gas).

Wait. Wouldn't we have to perform the casting at any rate?

That is, we would have to cast down from uint256 to uint128 when we save the variables in storage.

[smol-ninja]

Yes indeed. But when we use uint128 for struct memory variables, it has to load it into stack and use PUSH16 and AND to make it fit into uint256 slot. And then every time it references from the memory to load into stack, it has to perform those opcodes again because of data type.

I compared the opcodes, and it uses 3 additional PUSH16 and 3 additional AND opcodes hence 18 gas difference.

    struct StackParam {
        uint128 amount;
    }

    struct StorageParam {
        uint128 amount;
    }

    StorageParam public storageParam;

    function create() public returns (StackParam memory stackParam) {
        stackParam = StackParam({
            amount: 9
        });

        storageParam.amount = stackParam.amount;
    }

vs

    struct StackParam {
        uint256 amount;
    }

    struct StorageParam {
        uint128 amount;
    }

    StorageParam public storageParam;

    function create() public returns (StackParam memory stackParam) {
        stackParam = StackParam({
            amount: 9
        });

        storageParam.amount = uint128(stackParam.amount);
    }

The second one still uses 18 less gas than the first one.

I am going to study Huff this weekend anyways to get deeper understanding of stacks and memory.

[PaulRBerg]

when we use uint128 for struct memory variables, it has to load it into stack and use PUSH16 and AND to make it fit into uint256 slot ... I compared the opcodes, and it uses 3 additional PUSH16 and 3 additional AND opcodes hence 18 gas difference

oh wow

Unfortunately, there's a big problem with switching to larger types for the parameters. We would have to handle the cases when the inputs provided by users are larger than the types allowed by the storage values.

That is obviously not worth it and we have to stick with the same types (uint128, etc.)

[smol-ninja]

We would have to handle the cases when the inputs provided by users are larger than the types allowed by the storage values.

We chose uint128 with the rationale that in practice no token amount can be greater than $2^{128}$ (until its a memecoin, just saying, that is designed to have extremely large supply). So isn't it safe to simple cast uint256 to uint128 without any sanity check?

[PaulRBerg]

with the rationale that in practice no token amount can be greater than

That's an assumption, yes. But, by using uint128 in the params, we are forbidding any tokens with supplies greater than $2^{128}$. That is, no stream can be created with those tokens. But if we allow uint256, then we would, in principle, allow invalid streams to be created. And that's bad.

So isn't it safe to simple cast uint256 to uint128 without any sanity check?

No. The protocol must remain idempotent with respect to all known escape hatches.

In simpler words, there must not be any known way to break the create stream behavior. Otherwise, bad actors will find a way to exploit it, somehow, sometime.

It's the kind of thing that would definitely get reported in a security review.

[PaulRBerg]

Closing as a duplicate of #728.