Structure of CreateMerkleLockup event

[smol-ninja]

The emitted events during CreateMerkleLockup index the whole ConstructorParams struct. However, since topics can't hold more than 32 bytes (from docs), the whole struct bytes get hashed before being emitted as an indexed value. Thus, any of the parameters of the ConstructorParams struct cannot be searched directly on-chain. You need to know the values of all the parameters and then use the Keccak-256 hash in order to search for it.

Therefore, it does not make sense to index the whole struct. There are two options that we can consider:

O1: Full expansion

event CreateMerkleLockupLL(
    ISablierV2MerkleLockupLL indexed merkleLockupLL,
    address admin,
    IERC20 asset,
    string ipfsCID,
    string name;
    bytes32 merkleRoot;
    uint40 expiration;
    bool cancelable;
    bool transferable;
    ISablierV2LockupLinear lockupLinear,
    LockupLinear.Durations streamDurations,
    uint256 aggregateAmount,
    uint256 recipientsCount
);

This would increase the gas cost by 2466.

O2: Index admin and asset

event CreateMerkleLockupLL(
    ISablierV2MerkleLockupLL indexed merkleLockupLL,
    address indexed admin,
    IERC20 indexed asset,
    MerkleLockup.ConstructorParams indexed baseParams,
    ISablierV2LockupLinear lockupLinear,
    LockupLinear.Durations streamDurations,
    uint256 aggregateAmount,
    uint256 recipientsCount
);

This would increase the gas cost by 750 = 375 * 2 new topics. Note that we are still hashing baseParams to save on gas cost.

O3: Unchanged / remove baseParams

event CreateMerkleLockupLL(
    ISablierV2MerkleLockupLL indexed merkleLockupLL,
    MerkleLockup.ConstructorParams indexed baseParams,
    ISablierV2LockupLinear lockupLinear,
    LockupLinear.Durations streamDurations,
    uint256 aggregateAmount,
    uint256 recipientsCount
);

This imo is the cheapest way to emit event but it makes baseParams unusable. We may be better off not including it at all and saving 375 gas (1 less topic).

O4: Remove indexed from baseParams

event CreateMerkleLockupLL(
    ISablierV2MerkleLockupLL indexed merkleLockupLL,
    MerkleLockup.ConstructorParams baseParams,
    ISablierV2LockupLinear lockupLinear,
    LockupLinear.Durations streamDurations,
    uint256 aggregateAmount,
    uint256 recipientsCount
);

This would increase gas cost by 2466 but this makes the entire baseParams usable through emitted logs.

Comments welcome from @sablier-labs/engineers.

[razgraf]

Thank you for this analysis @smol-ninja! The issue was prompted initially by my subgraph integration.

On the "indexed" aspect - in general we don't use any tooling that searches events on-chain on-demand, so (outside of any third parties maybe using it) we don't take advantage of event filtering anyway.

On the "indexed tuple" aspect, the fact that "the whole struct bytes get hashed" as Shub explained in the OP also means that the baseParams tuple is completely unusable (as I cannot decode it in the event parser).

---

O3 unfortunately isn't very attractive as it renders our event parsing useless. We could, as an alternative, use web3 calls to get those parameters on demand - but it introduces a ton of complexity and time into the subgraph/indexing algorithm (we already do that for a couple of things like asset data, and it proved non-trivial).

O2 seems like a good middle-ground, but we should decide if indexed elements are important for us or not. If the answer is no, O4 seems more efficient.

O1 is fine too, has its advantages (not having tuples makes usage through tools like Etherscan and Tenderly easier) but is a bit more messy.

---

I'm leaning towards O4, but would like to hear your takes as well.

[smol-ninja]

I am fine with O4 as well. However, since we don't require indexing, should we remove indexed from merkleLockupLL as well and save 199 gas, unless you think there could be third parties searching on-chain.

[smol-ninja]

O1 is fine too, has its advantages (not having tuples makes usage through tools like Etherscan and Tenderly easier) but is a bit more messy.

O1 and O4 are equally messier in terms of data size. O1 has an advantage over O4 in the context of data display on Etherscan and Tenderly. I would prefer O1 over O4 since it's just straightforward. The gas usage would be the same between O1 and O4 if we remove indexed from admin and asset in O1.

[razgraf]

That's the point, I have no knowledge of third parties using or depending on this. The safe approach would be to keep at least one information indexed, and the most important one is merkleLockupLL - as it signals the campaign being officially created.

By the way, If we were to remove it, then the conversation of removing all indexed modifiers from the contracts would start making sense too.

@gavriliumircea for our backend we've been thinking of adding an event watcher. Do you think indexed parameters would affect us in any meaningful way? Or maybe it didn't come up as something meaningful yet.

---

Later edit: I'm leaning towards O1 too, for the sake of simplicity with frontend parsers and JS transaction encoding.

[smol-ninja]

Just few updates:

1. O1 is not possible without introducing a new struct or skipping some variables in the event. The full expansion leads to too many variables on stack i.e. the infamous stack too deep error.
2. O4 increases the gas cost by 2466 and not by 900 because of name and ipfsCID strings. Strings take much more than 32 bytes.

We can go with O4 if we need full details to be emitted.

[smol-ninja]

Resolved in

• feat: include stream duration in CreateMerkleLockupLT event #309.