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

make transaction id idempotency optional in LPI #223

Closed
michielbdejong opened this issue Jun 13, 2017 · 3 comments
Closed

make transaction id idempotency optional in LPI #223

michielbdejong opened this issue Jun 13, 2017 · 3 comments

Comments

@michielbdejong
Copy link
Contributor

A connector could tell its peers "if you send two transactions with the same txid, then I'll charge you twice". This means only the sender's plugin, as well as plugins that listen on ledgers which don't guarantee to trigger the same incoming_prepare event only once, will need to store a complete log of transactions and guarantee transaction id idempotency. Other plugins can gain a speed up and remove a bottleneck by treating each transfer in isolation, without comparing its txid to anything.

@emschwartz
Copy link
Member

What you're describing is not really making it optional. The sendTransfer function must enforce that the transfers are idempotent with respect to the transfer ID. However, in plugins where there is no underlying ledger I think it's fair to say that this uniqueness will only be enforced on the sending side. It would be good to call this out though in the plugin docs.

@michielbdejong
Copy link
Contributor Author

Right, transaction id's are a concern at the LPI level. We can relax the constraint so that no two transfers with the same id can be pending at the same time, but once a transfer is finalized, its id could be reused again.

If sendTransfer returns a promise for the fulfillment, then we can get rid of the transaction id
altogether, right? See https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/pull/53/files#r151100592

@emschwartz
Copy link
Member

It is removed from #347, can we close this now?

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

No branches or pull requests

2 participants