FCL lib as dependency

[obatirou]

Hi, I am trying to integrate FCL as a dependency to the P256 repo in order to take advantage of updates and future audits. However, I am facing a few problems preventing this:

1. All exposed functions in FCL_elliptic.sol / FCL_WebAuthn.sol use calldata but ERC-1271 is using memory
2. FCL_Elliptic_ZZ.ecdsa_verify / FCL_WebAuthn.checkSignature are not view functions

All functions use calldata but ERC-1271 is using memory

It is not the first case of this, see https://github.com/rdubois-crypto/FreshCryptoLib/issues/9 I saw that @rdubois-crypto proposed 2 solutions for the first problem:

• Deploy and call the lib: unfortunately the lib is implementing only internal functions. According to documentation of solidity (and internal tests with foundry), the lib will be embedded in the calling contract. So, in order to have an external lib called in delegatecall a user would need to fork and modify the repo to have at least a public or external function. Another solution would be to integrate the lib as is in a contract, expose the lib functions with wrappers, deploy the contract and call it. This is what is done in tests but every user integrating the lib would need to create a contract with wrappers. Unfortunately, those solutions would also incur an additional cost of deployment of the contract + a call.
• Fork and modify FCL to accept memory: it would introduce forks increasing the number of potential implementation errors from users.

So having the lib implementing an external / public function or having a function supporting memory would be some solutions. A benchmark could be done in order to ensure the most efficient method to choose for the first problem which if we resume are:

• integrate the lib as is with wrappers in a contract and deploy it
• external library with exposition of external functions (needs changes in FCL)
• expose a memory compatible function in the lib (needs changes in FCL)

Note: OZ use memory with internal functions for its libs

FCL_Elliptic_ZZ.ecdsa_verify is not a view function

For the second problem: a few functions in FCL_elliptic.sol are not view (or pure) due to several calls to the precompile 0x05. Those calls can be transformed in staticcall and functions could have the view (or pure) modifier. This work is already part of https://github.com/rdubois-crypto/FreshCryptoLib/pull/18 but a dedicated PR could make sense. Does this PR is ready to be merge ? Would you prefer a dedicated PR ?

Conclusion

What are your thoughts on those ? I am available to work on implementations of any of those solutions. Let me know if I missed / misunderstood something or if you have other solutions.

[jayden-sudo]

Maybe we don't need any memory or calldata for the interface:

Because the parameters are points, we only need to destructure uint256[], for example:

ecdsa_verify(bytes32 message, uint256 r, uint256 s, uint256 Q0, uint256 Q1)

But as @rdubois-crypto mentioned, "Please do not modify API, as the functions are integrated elsewhere." So forking a modified version may be reasonable.

However, with the support of ec_recover_r1, I believe people will tend to use ec_recover_r1 instead of ecdsa_verify or simply use ec_recover_r1 directly.

[rdubois-crypto]

One solution could be to add the extra functions with the required 'memory' keyword instead of calldata to the lib. (maybe in a different file). Not fan of code duplication, but if they stand side by side, no PR shall be accepted without each version being identical.

The aim of this lib is to be the fastest implementation available. Calldata induces a lower cost. There are also debates about assembly and extra tricks (formulaes are not canonic doubling and add). Tools like TC makes intensive use of asm, as any good cryptographic library (critical parts are allways asm).

[obatirou]

One solution could be to add the extra functions with the required 'memory' keyword instead of calldata to the lib. (maybe in a different file). Not fan of code duplication, but if they stand side by side, no PR shall be accepted without each version being identical.

The aim of this lib is to be the fastest implementation available. Calldata induces a lower cost. There are also debates about assembly and extra tricks (formulaes are not canonic doubling and add). Tools like TC makes intensive use of asm, as any good cryptographic library (critical parts are allways asm).

Thank you for your answer. I agree with the fact that adding extra functions with memory would duplicate a lot of code (FCL_Webauthn.soland FCL_elliptic.sol in this case) and increase maintainability work / potential errors. It could make sense though as the aim is to be the fastest implementation. Also deploy the lib with a wrapper contract around is another viable solution as I understood you are not open to changes in the API. Maybe even integrate this wrapper in FCL as it would be a common use case ?

Regarding the second point, about not having external functions of FCL_Elliptic_ZZ being view, would you prefer a dedicated PR ? Or the work done in https://github.com/rdubois-crypto/FreshCryptoLib/pull/18 is enough and ready ?

[rdubois-crypto]

I have just one question in the comment of the PR. Let Jayden confirm and modify it (or not) before merging.

[obatirou]

I have just one question in the comment of the PR. Let Jayden confirm and modify it (or not) before merging.

Perfect, thank you very much

[rdubois-crypto]

Close with adoption of 'view' and 'staticcall' PR by jayden.

(Switch to be discussed)