Extract constants

[MerlinEgalite]

Based on @Rubilmax proposal #114 (comment)

[makcandrov]

I'm not sure if batching together a collection of unrelated constants is a great idea. I believe it would be better to keep the constants close to the logic where they are used. However, I'm in favor of splitting more the contract into libraries & abstract contracts, so that the constants would not be directly in the main contract

[pakim249CAL]

I'm not sure if batching together a collection of unrelated constants is a great idea. I believe it would be better to keep the constants close to the logic where they are used. However, I'm in favor of splitting more the contract into libraries & abstract contracts, so that the constants would not be directly in the main contract

Are you suggesting that we have several constants files with prefixes describing their context? I'm not necessarily against that idea, but I don't think there are enough constants to justify a split quite yet. I think splits should be done if the number of constants becomes unmanageable and hard to look up just as a practical point.

[makcandrov]

No, I was suggesting keeping the constants in the libraries/contracts they are used.

For example, WAD should be in a WadMath library, along with all the related math functions involving WAD. Same for signature related constants. For example MAX_VALID_ECDSA_S should be in a library handling signatures only (along with the Signature struct and some other generic signature related functions).

As for other EIP712 constants, as they are specific to Morpho, I think they should be in an abstract contract Blue would inherit from, that would provide a function _checkSignature or smth like that.

[pakim249CAL]

No, I was suggesting keeping the constants in the libraries/contracts they are used.

For example, WAD should be in a WadMath library, along with all the related math functions involving WAD. Same for signature related constants. For example MAX_VALID_ECDSA_S should be in a library handling signatures only (along with the Signature struct and some other generic signature related functions).

As for other EIP712 constants, as they are specific to Morpho, I think they should be in an abstract contract Blue would inherit from, that would provide a function _checkSignature or smth like that.

Good points. I think keeping constants where they are use allows for a good separation of concerns when reading the contracts. I think we can do this in future PRs though, as I believe that separating constants out to a library is still an improvement off of the current code.

[MerlinEgalite]

Yes I agree with you it can be better, for now let's do this and let's try to answer #98 first

[Rubilmax]

@makcandrov

As for other EIP712 constants, as they are specific to Morpho, I think they should be in an abstract contract Blue would inherit from, that would provide a function _checkSignature or smth like that.

Good luck convincing @MathisGD & @QGarchery, it's been 2 weeks I'm trying now. If I'm not mistaken, main reasons are: centralized storage and minimalism. And if you succeed, I'm jealous.

[makcandrov]

My argument in favor of splitting the contracts would be modularity. It's always more pleasant to import a single library/contract that contains only the methods/constants needed. This benefits both us and integrators who may want to import only specific parts of the code (only the math functions, or only the signature-related functions, etc.)

[makcandrov]

I believe that separating constants out to a library is still an improvement off of the current code

[MathisGD]

I'm not sure if batching together a collection of unrelated constants is a great idea.

Agree with Maxime.

I believe that separating constants out to a library is still an improvement off of the current code.

Why ? Do you have tangible arguments ?

---

I'm not fundamentally against this split (not really disturb reading), but I sincerely don't understand why it's a good thing (plus the PR does not provide any explanation neither a link to a discussion on this).

[MerlinEgalite]

I'm not sure if batching together a collection of unrelated constants is a great idea.

Agree with Maxime.

I believe that separating constants out to a library is still an improvement off of the current code.

Why ? Do you have tangible arguments ?

I'm not fundamentally against this split (not really disturb reading), but I sincerely don't understand why it's a good thing (plus the PR does not provide any explanation neither a link to a discussion on this).

Here is the proposition from @Rubilmax #114 (comment)

[MathisGD]

Unless there is a reason/argument for this split, I suggest that we close the PR.

[MerlinEgalite]

?