Whats the reason why we use a struct in the mint() func rather than individual vars?

[hilmarx]

Question: Does the benefit of less visual code outweigh the disadvantage of forcing web3 devs to pass dummy values for the struct in the mint function? Do I miss any other advantages here?

I tried:

 // ================  MINTING ==============================================
    // Only pass _executor for self-providing users, else address(0)
    function mintExecClaim(
        uint256 id,  // set automatically by mintExecClaim
        address provider,   //  if msg.sender == provider => self-Provider
        address providerModule,  //  can be AddressZero for self-Providers
        address userProxy,  // set automatically to msg.sender by mintExecClaim
        address condition,   // can be AddressZero for self-conditional Actions
        address action,
        bytes memory conditionPayload,  // can be bytes32(0) for self-conditionalActions
        bytes memory actionPayload,
        uint256 expiryDate
    ) public override {
        // Smart Contract Accounts ONLY
        userProxy = msg.sender;

        // EXECUTOR CHECKS
        require(
            expiryDate <= now + execClaimTenancy,
            "GelatoCore.mintExecClaim: execClaim.expiryDate"
        );
        address executor = providerExecutor[provider];
        require(
            isExecutorMinStaked(executor),
            "GelatoCore.mintExecClaim: providerExecutor's stake is insufficient."
        );

        // PROVIDER CHECKS (not for self-Providers)
        if (msg.sender != provider) {
            string memory isProvided = "test";//isProvided(_execClaim, executor, gelatoGasPrice);
            require(
                isProvided.startsWithOk(),
                string(abi.encodePacked("GelatoCore.mintExecClaim.isProvided:", isProvided))
            );
        }

        // Mint new execClaim
        currentExecClaimId++;
        id = currentExecClaimId;

        // ExecClaim Hashing
        bytes32 execClaimHash = keccak256(abi.encode(
            id,
            provider,
            providerModule,
            userProxy,
            condition,
            action,
            conditionPayload,
            actionPayload,
            expiryDate)
        );

        // ProviderClaim registration
        execClaimHashesByProvider[provider].add(execClaimHash);
        // emit LogExecClaimMinted(executor, id, execClaimHash, _execClaim);
    }

It should compile without STD based on the number of vars it has. What is the reason we use a struct here, other than "less code visibly"?

I like the struct from visual point of view and think it's a big improvement here. But what I dislike is that it forces UIs to submit dummy values for: id & userProxy, even though we only set them in the function. To use structs in the other functions also makes sense, as we do not use any dummy values.

I am also not sure if Web3 developer find it trivial to work with structs yet. Inputting individual variables is imo easier here, or is it the same in ethers, that you simply pass the values like you would normally?

Are there any other benefits than a) STD avoidance and b) visual appearance that outweigh c) UIs having to pass dummy values?

[gitpusha]

1. Thing I noticed: I think you suffer from the "Unknown Soldity Code Formatter" problem (I raised an issue in vscode-solidity). You probably accidentally auto-format your solidity code. I didnt find any configs for a solidity autoformatter but the VS-code extension seems to do some weird stuff. So be careful not to auto-format your solidity files, since it breaks with the style guide.

UPDATE: Juan Blanco responded to my issue and apparently fixed the defaultFormatter thing. You can now disable it in the extension apparently.

See: https://github.com/juanfranblanco/vscode-solidity/issues/168#issuecomment-606600726

[gitpusha]

Initially I introduced the structs due to 2 reasons: 1) better code readability 2) avoid STD

Now that we are shortly before a code freeze we can test where we have STDs and where not.

I like the struct from visual point of view and think it's a big improvement here. But what I dislike is that it forces UIs to submit dummy values for: id & userProxy, even though we only set them in the function. To use structs in the other functions also makes sense, as we do not use any dummy values.

I already discussed this issue with ricmoo. It seems like for solidity you should have explicit values and it does not make sense to pass an undefined.

I agree with you that this is a nuisance. We have two options:

1) Give dapp developers the default placeholders

• execClaimId should be constants.HashZero
• userProxy should be constants.AddressZero

Everything else is technically modifiable and hence does not require a placeholder by default. To be honest I dont think this is a lot of friction. My suggestion would be:

• We make a JS class called execClaim that has the defaults for execClaimId and userProxy filled in.
• This class has a constructor that accepts values for all non-default value vars
• Devs can instantiate their execClaims easily by passing their non-defaults to this class constructor and they dont have to worry or know about the default placeholders.

2) We rewrite the mint function and make it accept a bunch of params.

We should discuss the trade-offs of each approach on the phone. I tend towards option 1) because it's trivial to implement in a developer-friendly way and it makes the whole Solidity code base more unified, readable, and less prone to STDs.

[gitpusha]

Outcome:

• Postpone till later and if time implement for this release.