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

Global Maker #48

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Global Maker #48

wants to merge 2 commits into from

Conversation

MarvinJanssen
Copy link
Contributor

This one will never work in the current state because the sender and receiver are asserted in the static calls. Apart from that, the signatures validate with the Global Maker contract. I opted to pass some information to the Global Maker when it is constructed on where the maker address (/ from address) for specific call data will be. I do this by providing a list of function signatures and corresponding byte offsets. It makes it rather flexible and helper functions could be created to add more signatures without having to redeploy the contract.

There are multiple ways to get this to work. I assume a quick solution could be to add a call recipient address to Order (or something), and when present, pass that to the static call instead of the maker. Huge downside is that we have to change Order, obviously. Another possible solution is abandoning this model and looking at a shared proxy contract via the registry perhaps.

Input is appreciated.

@cwgoes
Copy link
Contributor

cwgoes commented Apr 19, 2021

This one will never work in the current state because the sender and receiver are asserted in the static calls.

Do you mean e.g. that the static calls assert that a particular account sent tokens? This check can just be removed.

For example, this line can be safely deleted. I just included it in the original static contracts for clarity on what was happening, but the recipient of tokens doesn't actually care where they come from (and the sender shouldn't have permissions to send them from anywhere else unless the sender is this kind of global maker contract).

@MarvinJanssen
Copy link
Contributor Author

Static Market also makes those assertions:

checkERC1155Side(data,addresses[1],addresses[4],tokenIdAndNumeratorDenominator[0],call_amounts[0]);

I do not see how you could remove them. Do you not want to assert you are actually being sent the token requested? I guess you do not technically have to assert who the sender is but in following your draft PR, the global maker is going to be the maker on both sides, is it not?

@cwgoes
Copy link
Contributor

cwgoes commented Apr 30, 2021

I do not see how you could remove them. Do you not want to assert you are actually being sent the token requested? I guess you do not technically have to assert who the sender is but in following your draft PR, the global maker is going to be the maker on both sides, is it not?

You can allow anyone to be the sender, while asserting the right token and right amount. This will be easier to do with decoding calldata and asserting equality of particular fields instead of constructing calldata and asserting equality of the whole thing.

@0xdecapitator
Copy link

we have to assert the recipient of the tokens which in this case (global maker) not possible because in both orders its the contract .. and since users approve that contract so i think its not possible to skip this assetion !! a solution would be adding the owner in extra data or adding executer ( global maker ) in order schema and use it just in executeCall function and continue the flow as is !!
how do u think ?

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 this pull request may close these issues.

3 participants