Fix revocation accum sync when endorsement txn fails #3547
Conversation
jamshale
force-pushed
the
failed-revocation-entry-with-endorser
branch
2 times, most recently
from
4de0aca to
eb5fb83
Compare
jamshale
changed the title
jamshale
force-pushed
the
failed-revocation-entry-with-endorser
branch
from
2ed99c6 to
d08add8
Compare
Signed-off-by: jamshale <[email protected]> Refactor cognitive complexity Signed-off-by: jamshale <[email protected]> Add happy path unit test Signed-off-by: jamshale <[email protected]> Raise a responder None error Signed-off-by: jamshale <[email protected]> Remove unused duplicated funtion Signed-off-by: jamshale <[email protected]>
jamshale
force-pushed
the
failed-revocation-entry-with-endorser
branch
from
bf34599 to
6afa43a
Compare
|
Squash rebased and added a happy path unit test. |
|
|
I'm getting weird behaviour with the alice/faber demo. If I issue a credential and then request a proof, the proof validates. If I revoke the credential and then ask for a proof again, the proof fails (as expected). However if I issue a second (or third) credential and then ask for a proof, the proof still fails ( This is happening on the aca-py |
| # We know this needs endorsement | ||
| endorser_did, connection = await self._get_endorser_info() | ||
| rev_reg_record, recovery_txn = await sync_accumulator(session=session) | ||
| await create_and_send_endorser_txn() |
There was a problem hiding this comment.
If an endorser is not required (i.e.: the agent is set-up with higher privileges than author), how is this handled? In our scenarios we will likely only ever use an endorser to get transactions approved to be written on the ledger, but I think it may not be the only scenario?
There was a problem hiding this comment.
In this case it would go through the existing re-sync and re-try flow. Also the manual fix-revocation-state could be applied for no endorser, although it shouldn't be needed. https://github.com/openwallet-foundation/acapy/blob/main/acapy_agent/revocation/models/issuer_rev_reg_record.py#L336
There was a problem hiding this comment.
If the endorser_did isn't configured then aca-py should try to write the ledger transaction directly.
I wonder if the demo isn't selecting a valid credential anymore. I would think an automated test would have detected this scenario. I'll try and manually test without the demo. |
I verified that it's selecting a valid credential. I also tried manually deleting the revoked credential(s) and the proof still failed. |
That's not good... I'm not sure when this happened or why a test isn't catching it. |
I can look into this issue, it's not related to this PR (I tested it on the aca-py |
|
There was something merged just yesterday related to presentation. I'll try and isolate when this happened. |
When the endorsement transaction fails, create an event in the response handler. Check the rev_reg_defs on the ledger to find the problem entry. Create a recovery transaction and update the local wallet and re-send the transaction to the endorser.
Retry the same accumulator error 5 times and then log the error.
Note: The
fix-revocation-entry-stateendpoint with create transaction still doesn't create an endorsed transaction. I didn't want to try a change that for this commit which needs to be backported. Also anoncreds (did:sov) likely has the same problem and this does not attempt to fix it here. Only for indy revocation.