openwallet-foundation / acapy

ACA-Py is a foundation for building decentralized identity applications and services running in non-mobile environments.
https://aca-py.org
Apache License 2.0
418 stars 512 forks source link

ACA-Py should perform wallet/ledger operations transactionally - Use atomic transactions #3031

Open WadeBarnes opened 4 months ago

WadeBarnes commented 4 months ago

The following description was copied from here; https://github.com/hyperledger/aries-cloudagent-python/issues/2256#issuecomment-1589168883

With ACA-Py it's possible to perform writes to the ledger when it's guaranteed one or more of the operations required will fail, leaving the wallet (secure storage) or the ledger in a state where ACA-Py is unable to recover the record on one, the other, or both sides.

Cases in point:

These senecios typically start off with ACA-Py writing to it's wallet and then attempting to write to the ledger, and in some cases a tails server. If any of these operations are interrupted ACA-Py does not attempt to clean up the state, it just throws an error.

ACA-Py should wrap these operations in an atomic transaction to ensure the user has some guarantee that the records are written to both secure storage and the ledger or not at all. That could be as simple as deleting the newly written record from secure storage in the case of a write failure to the ledger, whatever the reason (TAA, ACAPY_READ_ONLY_LEDGER, or network issues). This would eliminate the need for all of prechecks and write failures would be inconsequential since there would be no erroneous records left behind.

ff137 commented 1 week ago

Greetings! What is the current state of this issue?

swcurran commented 1 week ago

AFAIK — this is an umbrella issue that is not itself actionable (there is no single “fix” for applying two-phased commits between the wallet and ledger across all interacations), but references and should spawn additional issues to address specific places where the ledger-wallet coordination can be improved. The two example issues in the description are what actually needs to be fixed and AFAIK, they have not been addressed. The only place where significant work has occurred is with “fixing” the wallet and ledger to match after a failed revocation registry accumulator update. That was done a couple of years back, and @jamshale is working on a PR #3299 that addresses an issue with the implementation outlined in #3281.

Not sure how this will ever get closed.