refactor: merkle campaign constructors

[smol-ninja]

Changelog

• Closes Separate out parameters of createMerkleInstant function #16

[andreivladbrg]

@smol-ninja since we decided to have separate factory for each merkle contract, is this PR still relevant?

[smol-ninja]

Yes. We are only splitting once factory contract into four. That does not mean we will not have the shares data structure. IMO, lets merge this PR and then split the factory on top of the changes.

[andreivladbrg]

but do we still need this:

Changing campaignName and shape to bytes32 has reduced the contract size by 3% (23,638 kB with 1000 runs).

i believe having it as string is better for UX

[smol-ninja]

i believe having it as string is better for UX

Thats a good question. Lets ask @sablier-labs/frontend team if they are OK having campaignName as bytes32. I am fine with both., Except ipfsCID, we have other strings returning as bytes32 as well. If the team is fine with the change, I am fine keeping it as bytes32 as it does not offer a bad UX that much and at the same time makes contract code more efficient. The UI will still show the campaign name as string and on etherscan, I do not think the campaign name being string offers much value at all.

[andreivladbrg]

well, why would we use bytes32 if size is no longer an issue?
the ipfsCID discussion we had yesterday was only relevant to solution 3 from your discussion (proxy pattern). but now, since we will have 3 factories, it's no longer relevant.

[smol-ninja]

well, why would we use bytes32 if size is no longer an issue?

Fair point. I will change it to string but I would want to do it through a separate PR / issue (may be bundle it with factory splitting PR). #58 is built on top of this PR, so changing it would add a lot of work to that PR as well.

Is that OK?

[andreivladbrg]

after checking the roadmap for the next airdrops iteration, this change will be the only Hard Breaking Change (when that issue was created we didn't have the classification)

thus, to make things easier for @sablier-labs/frontend team, maybe it would be better to leave them as they are (though there is no need for the shape parameter in Merkle Instant).

i will let you, @razgraf, decide whether we should move forward with this change or not.

Related discussions

1. Account for string in Create parameters #1 (comment)
2. Separate out parameters of createMerkleInstant function #16

[razgraf]

As discussed on Slack, we're okay moving forward with this.

[smol-ninja]

@andreivladbrg I have pushed the commit as we discussed. I have also rolled back shape and campaignName back to string datatype. I'd request you to review it now.

[smol-ninja]

@andreivladbrg can you please approve this PR if all looks good now?

[andreivladbrg]

@andreivladbrg can you please approve this PR if all looks good now?

sure, i haven't checked the inherited components, but i trust you on this