chore(protocol): fix inconsistent named variables in function returns

[adaki2004]

For me the pre-finding is a bit ambiguous. First (by the title) it suggests inconsistent use of named returns but also says "same return style in all of it is functions”, later the last sentence says “Consider being consistent with the use of named returns throughout the codebase.”, which also means (imo) that it is about named variables only (and not applicable where we don’t have a named variable in the returns).


so it can signal:

• Use the same naming convention (variable vs. _variable everywhere)
• Either use named variables (everywhere) or not use them (at all)


For me, the former makes more sense, since the latter would result in function body logic changes - a very simple example of that is: TestnetTierProvider getMinTier() -> If we are about to use named variables here, we should modify the if clause to be an if else clause, or adding extra return value to the if, otherwise the flow changes

So I tried to follow the former bullet point to make them consistent:

• Now, we do not use named variables where do not necessarily need them (e.g.: SgxVerifier addInstances() or SignalService sendSignal() ) And it also helps avoid adding new code logic (e.g.: TestnetTierProvider getMinTier() -> If we are about to use named variables here, we should modify the if clause to be an if else clause, or adding extra return value to the if, otherwise the flow changes)
• Where the return value is a tuple and one is named, the other is not, then decide based on practicality to either remove the naming, or add the name to the other unnamed variable. (E.g.: _verifyQEReportWithIdentity)
• To reach consistency across all named variables, r is applied as a prefix. Sometimes we had _variable (to not shadow other vars) and sometimes variable only but according to Solidity styling guide, _ shall be only used for private or internal state variables and non-external functions.

[adaki2004]

@davidtaikocha could you please check the failing genesis tests root cause ? I have no idea how these changes can cause failing them and i even reverted them to see if the renaming caused it, but seemingly not.

[dantaik]

I don't like the "rReturn" approach. Can we have the following convention:

function (type param) returns (type returnVal)

}

[adaki2004]

I don't like the "rReturn" approach. Can we have the following convention:

function (type param) returns (type returnVal)

}

So you mean having return prefix instead of r prefix ?

[dantaik]

Prefer this approach #16168, but I don't want to cause the relayer to change a lot of code again. So I'm not sure we should merge this BIG PR.