Suggestions for the MerkleStreamer API

[razgraf]

Parameters for the factory create method

v2-periphery/src/SablierV2MerkleStreamerFactory.sol

Lines 50 to 51 in 89aa8bd

	uint256 campaignTotalAmount,
	uint256 recipientsCount

Question: Why is campaignTotalAmount prefixed with campaign while recipientsCount isn't?
Suggestion: Could we somehow bundle these parameters, either by prefixing them similarly or (better) by grouping them under an attributes struct? I'm mostly thinking of those that are meant to be emitted for indexing, so: [CID, amount, recipients]. Alternatively, simply removing the campaign prefix would also be much less verbose.

Consistency in parameter order

v2-periphery/src/SablierV2MerkleStreamerFactory.sol

Lines 57 to 62 in 89aa8bd

	bytes32 salt = keccak256(abi.encodePacked(lockupLinear, initialAdmin, asset, merkleRoot, expiration));
	// Deploy the Merkle streamer with CREATE2.
	merkleStreamerLL = new SablierV2MerkleStreamerLL{salt: salt} (
	initialAdmin,
	lockupLinear,

I don't think it influences the salt in a bad way so could we maybe swap the order of lockupLinear and initialAdmin to mirror the one we use below for the new LL constructor?

[andreivladbrg]

Why is campaignTotalAmount prefixed with campaign while recipientsCount isn't?

It was done to avoid any potential confusion with the totalAmount parameter from the create function:

v2-periphery/src/SablierV2MerkleStreamerLL.sol

Line 92 in 89aa8bd

totalAmount: amount

We could add "campaign" prefix on recipientsCount as well, or just remove it. Both work for me. @PaulRBerg you decide 😅

[CID, amount, recipients]

So, are you suggesting something like this?

    struct EmitParameters {
        string ipfsCID;
        uint256 campaignTotalAmount,
        uint256 campaignRecipientsCount
    }

    function createMerkleStreamerLL(
        address initialAdmin,
        ISablierV2LockupLinear lockupLinear,
        IERC20 asset,
        bytes32 merkleRoot,
        uint40 expiration,
        LockupLinear.Durations memory streamDurations,
        bool cancelable,
        EmitParameters memory emitParameters
    )  external
       returns (ISablierV2MerkleStreamerLL merkleStreamerLL);

I don't think it influences the salt in a bad way so could we maybe swap the order of lockupLinear and initialAdmin to mirror the one we use below for the new LL constructor?

Good idea, we could do that.

[PaulRBerg]

Why is campaignTotalAmount prefixed with campaign while recipientsCount isn't?

@andreivladbrg answered this. Each stream's deposit amount is called totalAmount in V2 Core, so we added the "campaign" prefix as a qualifier to differentiate it from the stream amounts.

Rather than adding a superfluous prefix to recipientsCount (which doesn't need any "campaign" qualifier), what if we renamed campaignTotalAmount to aggregateAmount?

grouping them under an attributes struct?

Structs have bad UX for integrations (see: Tally integration on Telegram).

I suggest avoiding structs unless they are necessary (which isn't the case here because there are no StackTooDeep issues).

swap the order of lockupLinear and initialAdmin

sounds good - @andreivladbrg can you implement this change, please