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 tokenNetwork to transfer's unique/primary key/id #3096

Open
andrevmatos opened this issue Apr 4, 2022 · 0 comments
Open

Add tokenNetwork to transfer's unique/primary key/id #3096

andrevmatos opened this issue Apr 4, 2022 · 0 comments

Comments

@andrevmatos
Copy link
Contributor

andrevmatos commented Apr 4, 2022

Description

Internally, transfers are uniquely identified (primary key) by a string composed of 2 fields: {direction}:{secrethash}. This works fine if we think that, even for mediators, there may only be at most 2 transfers with same secrethash: one incoming (received), one outgoing (sent); this has worked fine up to now; but if we think about a multi-tokenNetwork setup (and maybe in the future even a multi-network client), specially in the context of atomic-swaps, this gets in the way: atomic swap mediators (if it happens that the same mediator is mediating both token's transfers) may see 2 pairs of transfers with same secrethash: incoming-outgoing for tokenA, incoming-outgoing on the reverse direction for tokenB.

Therefore, to pave the way and allow for easily supporting atomic token swaps, we must ensure the client can differentiate transfers per tokenNetwork (which also ensure they can be uniquely identified on different networks/deployments too).

Since all these information are already present on transfer's state, it should be trivial to migrate the database and do this in a fully-backwards-compatible manner.

Acceptance criteria

  • tokenNetwork is added as part of transfer's primary key identifier

Tasks

  • [ ]

Why do this?

Makes the schema layout more robust (makes transfer key more specific and less chance of a conflict which is probable in e.g. case of atomic swaps if that were ever to be implemented) for future changes, this is a refactoring that should have been done regardless.

@andrevmatos andrevmatos added this to the Tech Debt milestone Apr 4, 2022
@andrevmatos andrevmatos changed the title Change transfer's unique key/id to include tokenNetwork Add tokenNetwork to transfer's unique key/id/primary key Apr 4, 2022
@andrevmatos andrevmatos changed the title Add tokenNetwork to transfer's unique key/id/primary key Add tokenNetwork to transfer's unique/primary key/id Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant