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

feat(relay): report double payment #171

Merged
merged 3 commits into from
Jul 14, 2021

Conversation

gregdhill
Copy link
Member

@gregdhill gregdhill commented Jul 8, 2021

@gregdhill gregdhill marked this pull request as ready for review July 8, 2021 16:32
@gregdhill
Copy link
Member Author

It might be cleaner to store the Bitcoin txId on redeem / replace / refund execution and remove this new function. What are your thoughts @sander2 @nud3l?

@sander2
Copy link
Member

sander2 commented Jul 9, 2021

It might be cleaner to store the Bitcoin txId on redeem / replace / refund execution and remove this new function. What are your thoughts @sander2 @nud3l?

As mentioned on discord, if a vault makes 2 payments, and then uses the second payment for execute, the first block must be checked and reported by the clients after the execute.

@gregdhill
Copy link
Member Author

Either way this will be difficult to implement client-side, but the advantage of the alternate approach is that the data model is cleaner, each request is strongly linked to a single transaction ID and we would only require one function call to report theft.

@sander2
Copy link
Member

sander2 commented Jul 12, 2021

Either way this will be difficult to implement client-side, but the advantage of the alternate approach is that the data model is cleaner, each request is strongly linked to a single transaction ID and we would only require one function call to report theft.

But it wouldn't be able to catch everything. What if the vault just makes multiple payments without ever executing the redeem?

@gregdhill
Copy link
Member Author

Either way this will be difficult to implement client-side, but the advantage of the alternate approach is that the data model is cleaner, each request is strongly linked to a single transaction ID and we would only require one function call to report theft.

But it wouldn't be able to catch everything. What if the vault just makes multiple payments without ever executing the redeem?

Good point, it would need to be executed before we could mark it for theft which isn't ideal - perhaps this is the optimal solution.

@gregdhill gregdhill requested a review from sander2 July 12, 2021 16:07
crates/relay/src/lib.rs Outdated Show resolved Hide resolved
crates/relay/src/lib.rs Outdated Show resolved Hide resolved
crates/relay/src/lib.rs Outdated Show resolved Hide resolved
crates/relay/src/lib.rs Show resolved Hide resolved
crates/relay/src/lib.rs Show resolved Hide resolved
crates/relay/src/tests.rs Show resolved Hide resolved
Copy link
Member

@nud3l nud3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach. You could use the storing of the BTC tx id on redeem in combination flagging it as theft if it has not been executed after the redeem period expired. But I think this is cleaner since it does not introduce extra logic in redeem and kind of shifts complexity to the clients.

crates/relay/src/lib.rs Show resolved Hide resolved
crates/relay/src/lib.rs Show resolved Hide resolved
crates/relay/src/lib.rs Show resolved Hide resolved
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@nud3l nud3l merged commit c6c4dc6 into interlay:master Jul 14, 2021
@gregdhill gregdhill deleted the feat/report-double-payment branch July 14, 2021 11:37
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.

None yet

3 participants