AthanorLabs/atomic-swap

⚡ Solidity Improvements

0xClandestine opened this issue · 3 comments

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
        )
    );
}

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

noot commented

@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 :)

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