Align expected protocol states with RFCs

[hyperledger]

Align expected protocol states with RFCs

[swcurran]

Absolute agreement that the RFC states need to be used. @nodlesh this will probably mean that tests and steps need to be changed and the ACA-Py backchannel as well. And perhaps, ACA-Py needs to change -- that's TBD.

Suggest that someone -- @TimoGlastra (?) -- create a google spreadsheet that has:

• a section per protocol as you start to work on the protocol
• the states of the RFC
• the states in the Test Harness
• highlighted differences

With each protocol so documented, Sheldon will change the Test Harness and ACA-Py backchannel, update the spreadsheet (and notify us the change has been made) and if appropriate, discuss with the ACA-Py team if ACA-Py should be changed.

Open to other ideas on the approach.

This is very good progress -- making the tests truly aligned with the RFCs.

Thanks!

[TimoGlastra]

Awesome. I'll look into creating the spreadsheet tomorrow

[nodlesh]

Yes, definitely agree on changing the test harness and maybe the aca-py backchannel. We've mentioned this before, there may even be an issue logged on it. The AATH has to align to the states in the RFC. IMO, Aca-py needs to change the states as well to what the RFC mentions. If the decision is not to change Aca-py then the Aca-py backchannel will swap out the states in the responses for what the test harness expects.
I can make this a priority, after I have the Present Proof Test Scenario I'm working on completed.
If I'm not mistaken, the Issue Cred and Present Proof 1.0 RFCs don't actually mention what the states should be in those protocols. We have to make a decision to either remove the state checks in the Tests, keep the states in the tests and let the backchannels translate, or add states to the RFC. I'd recommend the RFCs mention the states so we have some consistency.

[TimoGlastra]

You're right @nodlesh. The 1.0 RFCs don't include the states. Hadn't noticed that.

We have to make a decision to either remove the state checks in the Tests,

Interesting you bring this up. I had a discussion with Tomislav yesterday where he said:

The problem with the states is that they are mentioned in the RFC, but are not required or part of the messaging protocol.
They feel like recommendation
So in my opinion, they shouldn't be part of the interop test harness

This was specifically about the connection protocol, but it would apply to all protocols. However even if we remove them from the test harness I think we'll need to add in something that will eventually do the same: check in which step of the protocol we are. e.g. Is it correct 'agent A' send a message to 'Agent B', and did 'Agent B' indeed receive this message. It however is not related to the state anymore then, but related to the actions that happened.

[nodlesh]

My initial readings of the Connection RFC didn't feel like the states were recommendations to me, but I can see how it could be construed that way. If the tests have no state to check, the assertions get pretty thin. Agents must be maintaining some sort of protocol state internally, so it would be good to pass that back to the backchannel. The tests can continue to check for a state, the backchannels can translate whatever state is received from the agent to what the tests expect, if the state is equivalent.
In general the tests at each step do the following:

• Call the operation
• Assert the response code
• Assert the status of the caller in the response
• Save off any needed items in the response data to the test context
• Assert the status of the receiver in the web hook.

[swcurran]

In the initial versions of RFCs, states were included but may not have been seen as required. We have tried to make it very clear that protocols are all about state management and that a clear definition of states is absolutely crucial in a protocol RFC.

While I can abide by Tomislav's comments in the first few RFCs that have been implemented, I disagree with that view ongoing. I definitely disagree that the interop test suite should do anything other than to take them as truth. I'm OK with the backchannel masking an RFC-implementation difference for now, but I would strongly encourage the builders to eliminate the need for that over time. Basically -- I encourage the proposal to Sheldon above: manage the state differences in the acapy backchannel for now, but feed back to the ACA-Py team to fix the state management in the framework.

[TimoGlastra]

@swcurran @nodlesh here is a spreadsheet that gives an overview of all the states: https://docs.google.com/spreadsheets/d/1ViG4swP3-0X3_a0Ak_6eGqzhCZomvlJeb4uj-DSh4Jo/edit?usp=sharing

Colors automatically change based on if it matches the RFC expected state. @swcurran Is this like you had it in mind?

[TimoGlastra]

I think this one can be closed now. Great :)