Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add from, to, and userData to TradingStrategy interfaces #77

Closed
nventuro opened this issue Oct 30, 2020 · 3 comments · Fixed by #89
Closed

Add from, to, and userData to TradingStrategy interfaces #77

nventuro opened this issue Oct 30, 2020 · 3 comments · Fixed by #89
Assignees

Comments

@nventuro
Copy link
Contributor

The TradingStrategy interface needs to be augmented with the following fields:

  • from: the address that is providing the funds (either directly or via user balance)
  • to: the address funds will be sent to (either directly or via user balance)
  • userData: arbitrary data provided by the caller of swapBatch

from and to are required to implement certain desired features, such as whitelists. We need for the Vault to pass them so that Trading Strategy can trust these values are correct.

userData opens the door for alternative TS designs, such as only allowing a trade if it is accompanied by a signature from a trusted third party.

@dmf7z
Copy link
Contributor

dmf7z commented Nov 2, 2020

I would also add the Vault's swap function caller because is does not always equals from.
This is necessary because a Trading Strategy may have to trust the origin of userData that could be a specific Trading Script smart contract which is caller and not always from.

@dmf7z dmf7z self-assigned this Nov 2, 2020
@nventuro
Copy link
Contributor Author

nventuro commented Nov 2, 2020

I'm not sure that's a direction we'd want to go in, why would a TS be restricted to a specific trade script? It seems overly restrictive.

@dmf7z
Copy link
Contributor

dmf7z commented Nov 2, 2020

TS needs to whitelist one or many trade scripts in case userData needs some level of trust, avoiding any other caller to manipulate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants