FacetCut struct in IDiamondWriteableInternal doesn't follow the Spec naming conventions

[zdenham]

The naming convention for the FacetCut struct does not follow the diamond spec, which uses a different naming convention facetAddress and functionSelectors:

solidstate-solidity/contracts/proxy/diamond/writable/IDiamondWritableInternal.sol

Lines 23 to 28 in ae15a06

	struct FacetCut {
	address target;
	FacetCutAction action;
	bytes4[] selectors;
	}
	}

This could potentially break client compatibility across diamonds. E.G. when I was porting over an ethers diamondCut script from another project, there were type errors with the SS Diamond.

This PR updates the codebase and tests to match the standard naming conventions.

I actually like the SS naming convention better, but it might be worth maintaining consistency with the standard.

EDIT: this also isn't a problem for everyone: I tested with louper, and their diamondCut still works with SS diamond despite using the spec naming convention, *because the ABI for the diamond they are referencing uses the same convention.

https://github.com/mark3labs/louper-v2/blob/d326c9ba4c48724297a6b8f5dcc9915d148ed250/src/lib/components/AddFacet.svelte#L41-L47

[ItsNickBarry]

Sorry for the delay.

My first response was to say that the variable names are not part of the spec because they have no effect on ABIs. I disagree with several naming choices in EIP 2535 (as well as most other EIPs) and have made changes where possible. However, I've been simultaneously looking at EIP-712 (#188), which disturbingly uses variable names in its signature generation scheme.

So, I'm torn between making use of the freedom which should be allowed by the ABI system, and trying to accommodate an EIP that pretends like such freedom does not exist.

[zdenham]

Forgot about this 😅

variable names [...] have no effect on ABIs.

The encoded payload ends up being the same, but when updating a struct variable name and re-compiling the build artifacts / ABI are modified accordingly, which affects generated typechain types and validation at ethers.js level.

This was a minor inconvenience for me because I had a strongly typed diamondCut function (in typescript) as a part of a small internal library that broke when using SS ABIs because it expected "standard" naming convention. Was a design flaw of the lib mostly though, and a quick fix. But that is where the idea to update this came from.

That being said, if no one else has mentioned / run into similar issues since its probably not worth changing. Down to close this for cleanliness

[ItsNickBarry]

I think I'm going to end up merging. I'm just having a hard time accepting the implications.