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

⚡ Solidity Improvements #420

Open
0xClandestine opened this issue Apr 25, 2023 · 3 comments
Open

⚡ Solidity Improvements #420

0xClandestine opened this issue Apr 25, 2023 · 3 comments

Comments

@0xClandestine
Copy link

Hey guys, been watching the project unfold for months and have learned a lot. As a way to give back I thought I'd contribute some improvements for the smart contracts. Since I'm a first time contributor I feel its probably best to start with an issue outlining some of my improvements.

Improvement 1 ⛽

  • Tightly pack the swap struct.

Reasoning:

  • Reducing the amount of data that's hashed (via keccak256) will save a decent amount of gas. Given we're hashing the struct and not storing it there's an incentive to reduce the struct size as much as possible. Below you can see I've gotten it down to 5 slots, however we could pack it further.
// 256 + 256 + 160 + 160 + 160 + 128 + 64 + 64 + 32 = 1280 bits = 5 words (slots)
struct Swap {
    bytes32 pubKeyClaim;
    bytes32 pubKeyRefund;
    address payable owner;
    address payable claimer;
    address asset;
    uint128 value;
    uint64 timeout0;
    uint64 timeout1;
    uint32 nonce;
}

Improvement 2 ⛽

  • Use abi.encodePacked() rather than abi.encode() & unpack the struct from memory before encoding.

Reasoning:

  • Similarly to Improvement 1, the idea here is to reduce the amount of data that is being hashed to compute swapID. When using abi.encodePacked the returned output is tightly packed rather than including unnecessary padding. However as you may know abi.encodePacked is not supported for structs; I would suggest unpacking the Swap before encoding.
function computeSwapIdentifier(Swap memory swap) 
    internal 
    pure 
    returns (bytes32) 
{
    return keccak256(
        abi.encodePacked(
            swap.pubKeyClaim,
            swap.pubKeyRefund,
            swap.owner,
            swap.claimer,
            swap.asset,
            swap.value,
            swap.timeout0,
            swap.timeout1,
            swap.nonce
        )
    );
}
@0xClandestine
Copy link
Author

There's more improvements to come, just pushing what I had time to document last night.

@noot
Copy link
Collaborator

noot commented Apr 26, 2023

@0xClandestine hey, thanks for these suggestions! let us know if you plan to open a PR with these changes, otherwise I can add these in. if you have any contributing questions feel free to ask :)

@0xClandestine
Copy link
Author

@noot Would be glad to make a PR, will shoot for this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants