orbland / contracts

🔮 Orb and related contracts. Auction + Harberger taxed ownership + invocations.
https://orb.land
MIT License
22 stars 5 forks source link

[GAS OPTIMIZATION] trigger cleartext and hash is not required to be stored #20

Closed odyslam closed 1 year ago

odyslam commented 1 year ago

Description

When trigger() is invoked by the holder, they can submit cleartext and hash, with cleartext being optional. The cleartext and contentHash need not to be stored.

The reason is that the storage happens so that there is an immutable record of that trigger and its question. It's a social contract. That means that storage is not required, but rather just the fact that the hash and/or cleartext was submitted.

That means that even if the hash were submitted as a calldata argument without actually being used, the requirement for proof would be met with minimal gas implications. The calldata are part of the transaction and, thus, impossible to forge or alter.

Suggestion

Simply accept the data as calldata and emit the hash for ease of use by 3rd party apps and interfaces that build on top of the orb. The increment of the storage variable triggersCount can happen in the event definition. triggersCount++ will return triggersCount and then increment it.

function triggerText(string memory text) external onlyHolder onlyHolderSolvent {
    if (block.timestamp < lastTriggerTime + COOLDOWN) {
        revert CooldownIncomplete(lastTriggerTime + COOLDOWN - block.timestamp);
    }
    lastTriggerTime = block.timestamp;
    emit Triggered(msg.sender, triggersCount++,  keccak256(bytes(text)), block.timestamp);

}
function triggerHash(bytes32 contentHash) external onlyHolder onlyHolderSolvent
{
    if (block.timestamp < lastTriggerTime + COOLDOWN) {
        revert CooldownIncomplete(lastTriggerTime + COOLDOWN - block.timestamp);
    }
    lastTriggerTime = block.timestamp;
    emit Triggered(msg.sender, triggersCount++, contentHash, block.timestamp);
}
lekevicius commented 1 year ago

We don't need to store cleartext (that can forever live as just calldata), but we want to store hash so cleartext could be added later via recordTriggerCleartext().

odyslam commented 1 year ago

Then, the suggestion is modified as follows:

    if (block.timestamp < lastTriggerTime + COOLDOWN) {
        revert CooldownIncomplete(lastTriggerTime + COOLDOWN - block.timestamp);
    }
    lastTriggerTime = block.timestamp;
    triggers[triggersCount] = contentHash;
    emit Triggered(msg.sender, triggersCount++, contentHash, block.timestamp);