Bytes params for vesting factory

[arr00]

No description provided.

[netlify]

✅ Deploy Preview for confidential-tokens ready!

Name	Link
🔨 Latest commit	c8f819c
🔍 Latest deploy log	https://app.netlify.com/projects/confidential-tokens/deploys/6891d986a6a145000804e401
😎 Deploy Preview	https://deploy-preview-135--confidential-tokens.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[james-toussaint]

Filtering events by beneficiary would be useful for developers.

[arr00]

agree but that is part of the initialization. Do you propose passing it separately as well just for the sake of indexing?

[arr00]

I think integrators can look at the initialization code

[james-toussaint]

Should we:

1. provide this as ther default behavior in the factory
2. and same with return address(new VestingWalletCliffExecutorConfidential());
3. And add that piece of code to documentation:

    function _deployVestingWalletImplementation() internal virtual override returns (address) {
        return address(new MyCustomVestingWalletConfidential());
    }

    function _validateVestingWalletInitialization(bytes memory initialization) internal virtual override {
        (
            address arg1,
            uint48 arg2,
        ) = abi.decode(initialization, (address, uint48));
        require(arg1 != address(0));
    }

[arr00]

We can't provide default behavior for _deployVestingWalletImplementation due to the config (chain and protocol specific). If we won't have a default for one, I don't think there should be a default for the other.

[james-toussaint]

You are right about config yes, hence keeping both is best yes 👍

[james-toussaint]

It's just a mock but maybe we could remove duplication here

[arr00]

duplication?

[james-toussaint]

I was think about that repeated twice:

abi.decode(initArgs, (address, uint48, uint48, uint48, address));