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

Feature Request: Consider adding packed keyword to pack struct fields #13175

Closed
Pzixel opened this issue Jun 20, 2022 · 4 comments
Closed

Feature Request: Consider adding packed keyword to pack struct fields #13175

Pzixel opened this issue Jun 20, 2022 · 4 comments
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. feature stale The issue/PR was marked as stale because it has been open for too long.

Comments

@Pzixel
Copy link

Pzixel commented Jun 20, 2022

Abstract

Consider following struct:

struct Foo {
    bool val1;
    bool val2;
    bool val3;
    bool val4;
}

When abiencoded value Foo(true, true, false, true) translates into 0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001, while it could be just 1101.

Motivation

Since gas usage is a big topic almost all big ethereum projects use tight structs packing to save caller funds. You can see it in the wild everywhere, e.g. 1Inch limit orders encodes order Id and timestamp into Order.info or Uniswap V3 swaps encodes zeroForOne and oneForZero fees into feeProtocol field. There are other examples in the wild. Since it's a repeatable pattern which is kinda error-prone and dull I think it just might be worthwhile to move this to compiler and code generation.

Specification

Introduce new keyword packed that changes ABI representation and field access. It might look like:

struct packed Foo {
    bool val1;
    bool val2;
    bool val3;
    bool val4;
}

This struct should be only 4 bytes long. When abiencoded it might look like 00000000000000000000000000000000000000000000000000000000000001101 or 11010000000000000000000000000000000000000000000000000000000000000 depending on if we want bytes or uint zeroing pattern. Compiler is responsible for generating appropriate bit shifts and masks when reading and writing fields:

foo.val2 = true; // foo = foo & (1 << 1)

Note that unlike current behavior this should affect calldata representation as well as inner storage.

Backwards Compatibility

Since this proposal introduces a new keyword it doesn't change meaning for any existing code.

Related: #11691

Additional considerations

It may be also worthwhile to consider packed arrays like:

function bar(uint8[] packed calldata foo) external {
}

so it doesn't put padding zeroes between elements, but unaligned access from those might be tricky and come with surprise costs.

@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 30, 2023
@Pzixel
Copy link
Author

Pzixel commented Mar 30, 2023

Hi so ideas on all this?

@github-actions github-actions bot removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 31, 2023
@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jun 30, 2023
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Jul 7, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. feature stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

2 participants