Add solvers #43
Add solvers #43
Conversation
| "attributes": { | ||
| "name": { | ||
| "type": "string", | ||
| "required": true | ||
| }, |
There was a problem hiding this comment.
Can we add solverId field?
That should be unique and match the ids stored on the backend, to avoid another mapping on the frontend.
We could maybe rename this one to displayName and make it not required.
There was a problem hiding this comment.
Can we add solverId field?
No problem
We could maybe rename this one to displayName
Fine with me, but why not just name?
and make it not required.
Why? I would hope they all have name, right?
There was a problem hiding this comment.
Just name is fine.
I thought about being explicit it's not the same name/id used on the backend, which is for display purposes.
And I proposed to make it not required because we can use the solvedId in case one is not provided.
| "default": 0, | ||
| "type": "integer" |
There was a problem hiding this comment.
Why an integer?
Sorry if I gave that impression.
My idea with this field is to match the ids used in the backend, which are actually strings.
For example, there's a solver named gnosis_1inch.
I want to use that as the id here, as for the name we could have Gnosis 1inch solver (that's why I proposed to change it to displayName.
There was a problem hiding this comment.
I see, I can call it displayName no worries. What do I call this solverId? you are happy with solverId (normally ID are UUID or numbers so might be confusing). Alternative I could call this the name but might also be confusing.
There was a problem hiding this comment.
I don't have anything better.
Yes, not numeric neither UUID, but still a unique identifier of the solver that's not meant to be nice for displaying.
Networks
Allow to add our supported networks to the CMS.
Solver
Model solvers.
After this PR
After merging the PR we should make a new step in the process to add new networks
Test
Run locally, and try to create the networks and a solver