0

I have an escrow from which addresses can withdraw from after a timelock. Each address (1) has a balance, (2) can request to withdraw from at any time, and (3) actually withdraw from at a later date. I have three separate data structures to manage requests and withdrawals:

// mapping(address => mapping(timestamp when funds will be withdrawable => funds)
mapping(address => mapping(uint256 => uint256) private requests;

// mapping(address => array of timestamps at which funds will be withdrawable
mapping(address => uint256[]) private timestamps;

// mapping(address => mapping(timestamp => whether the timestamp has been added to 'timestamps'
mapping(address => mapping(uint256 => bool)) private timestampLookups;

When an address requests a withdrawal, they can call:

function requestWithdrawal(uint256 amount) external {

    // The timestamp at which funds will be withdrawable
    uint256 timestamp = block.timestamp + 2000000;

    // Increment the amount that will become withdrawable for the given timestamp
    requests[msg.sender][timestamp] += amount;

    // Add the timestamp to `timestamps` if not already a member
    if (timestampLookup[msg.sender][timestamp] == false) {
        timestampLookup[msg.sender][timestamp] = true;
        timestamps[msg.sender].push(timestamp);
    }
}

After the lockup elapses, an address can now withdraw its funds by calling the following:

function withdraw(uint256 amount) external {

    // The sum of withdrawable tokens spanning all timestamps
    uint256 amount;

    // Loop over 'timestamps'...
    for (uint16 i = 0; i < timestamps[msg.sender].length; i++) {

        // ... to get the timestamp of each request
        uint256 memory timestamp = timestamps[msg.sender][i];

        // Only proceed if the timestamp has expired
        if (timestamp <= block.timestamp) {

            // Then add the value of the request for the timestamp
            amount += requests[msg.sender][timestamp];

        } else {

            // see below for idea of what to do here

        }
    }

    // Finally, transfer 'amount'
    require(myToken(msg.sender, amount));
}

In the withdraw for loop, I need to loop over timestamps to access the cumulative withdrawable value across all timestamps in requests.

The problem is that I need to confirm the token.transfer succeeds before mutating requests, timestamps, and timestampLookups, which would ideally be done in the for loop.

Rather than iterate over the same data twice, my idea is to create a uint256[] memory newTimestamps inside function withdraw and add the timestamps that have not expired (in the else block). Then, if require(myToken(msg.sender, amount)); returns truthy, simply reassign timestamps[msg.sender] to newTimestamps, at which point the timestamps that have been withdrawn from would be effectively deleted. If the transfer fails, don't do any reassignment.

Would it be good practice to not zero out values from the other two data structures (requests and timestampLookups) and leave them as is, knowing (1) those keys will never be accessed again and (2) to only and always rely on timestamps as the single source of truth for pending withdrawals?


PS There is logic in these functions to require that there are enough funds in the address' balance for withdrawal that I removed for simplicity.

  • 1
    The answer to the question in the title is: Yes, of course it's ok to not zero out old mapping keys to save gas. It's also ok to send all your ether to my account. – goodvibration May 17 at 6:49
  • @goodvibration are there any storage cost implications to be aware of for the old keys, or would leaving them alone still be (significantly?) cheaper than O(n) rewrites? – slider May 17 at 6:54
  • 1
    It's those rewrites (of zeros) which give you back the gas! – goodvibration May 17 at 6:55
  • Ah, right. I may create an array of expiredTimestamps and if the transfer succeeds, iterate over its length to delete requests[msg.sender][expiredTimestamps[i]] and timestampLookups[msg.sender][expiredTimestamps[i]] to free some garbage and get some gas. Would the delete operations be worth the extra loop(s)? I guess it depends on the num of loops. – slider May 17 at 7:15
  • Yes, just as long as you do it from a transaction that has already cost you some gas, because gas refund is limited to half of the cost of the entire transaction. So, for example, if you add a standalone function which does nothing but zerofying all these storage slots, calling it is just gonna cost you. Side note: I wouldn't call it "free some garbage" because it's not in the same sense as freeing allocated memory in "traditional" systems. – goodvibration May 17 at 7:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.