Declutter issuance&presentation protocols

[Patrik-Stas]

Changes

• No breaking changes
• Remove concept of "Action" as way to trigger actions/process messages from both Holder and Issuer handlers & state machines
• Externalize interactions with mediated connections and related code to files mediated_holder.rs, mediated_issuer.rs etc. out of state machine/handlers code itself
• Removed disabled unit tests - majority of tests was testing validity of sequence of FSM transitions. These will be irellevant with typestate pattern

Next steps:

• We can follow this up with changes to states to remove IO - that itself has higher value than transformation to typestate pattern.
  • After that, we can in parallel boost up testing performance significantly Remove IO between the parties in protocol integration tests #786
• In last stage we could rewrite thing to typestate + abstracting out & unifying approach for message processing as drafted in Proposal/processor #944

[codecov-commenter]

Codecov Report

Merging #945 (8f51797) into main (37a3130) will increase coverage by 0.19%.
Report is 1 commits behind head on main.
The diff coverage is 66.71%.

❗ Current head 8f51797 differs from pull request most recent head 3c7a59e. Consider uploading reports for the commit 3c7a59e to get more accurate results

@@            Coverage Diff             @@
##             main     #945      +/-   ##
==========================================
+ Coverage   44.25%   44.44%   +0.19%     
==========================================
  Files         418      417       -1     
  Lines       29311    28974     -337     
  Branches     6262     6177      -85     
==========================================
- Hits        12972    12878      -94     
+ Misses      12513    12297     -216     
+ Partials     3826     3799      -27     

Flag	Coverage Δ
unittests-aries-vcx	44.44% <66.71%> (+0.19%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
aries_vcx/src/handlers/util.rs	17.64% <0.00%> (+3.18%)	⬆️
aries_vcx/src/protocols/issuance/mod.rs	0.00% <ø> (-15.00%)	⬇️
aries_vcx/src/handlers/issuance/issuer.rs	44.17% <17.14%> (-9.82%)	⬇️
...ries_vcx/src/handlers/proof_presentation/prover.rs	44.26% <20.00%> (-11.99%)	⬇️
...es_vcx/src/handlers/proof_presentation/verifier.rs	50.73% <27.27%> (-6.09%)	⬇️
...vcx/src/protocols/issuance/issuer/state_machine.rs	49.22% <30.00%> (+0.15%)	⬆️
...otocols/proof_presentation/prover/state_machine.rs	45.20% <38.70%> (+13.67%)	⬆️
aries_vcx/src/handlers/issuance/mediated_holder.rs	41.07% <41.07%> (ø)
...src/handlers/proof_presentation/mediated_prover.rs	44.26% <44.26%> (ø)
aries_vcx/src/handlers/issuance/mediated_issuer.rs	46.55% <46.55%> (ø)
... and 9 more

... and 1 file with indirect coverage changes

[gmulhearn]

i'm not too familar with the usage of the actions flows to fully review... so consider this as a half-review. but lgtm other than above comments

[Patrik-Stas]

@gmulhearn okay, just for extra context, those "actions" were an obscure way to interact with state machines. For both processing incoming messages and also triggering "local actions" - for example sending response to counterparty.

So where previous you would have
enum Action { AcceptOffer(offer_message) SendRequest() }
and universal method

fn update(action: Action) {
   match (action) { 
      AcceptOffer(msg) => self.accept_msg_offer(msg)
      SendRequest() => self.send_request()
   }
} 

(in fact self.accept_msg_offer(msg) didn't really exist on top layer, but usually these matches were delegated to the most bottom layer - the state machine structure itself.)

now we simply have every "action" replaced by a appropriate method on both handler and state machine level. There's no more universal update method which might-or-might-not do 10 different things, based on what the action variant is.

[bobozaur]

Are there any plans regarding removing stuff like this? IMO the protocol should have nothing to do with finding messages and even if we don't remove the mediated connection stuff, perhaps we can separate concerns a bit better?

[Patrik-Stas]

Definitely, it's only exists because:

• update_state functions in libvcx_core, and these are also relied on in NodeJS tests
• integration tests for aries-vcx itself - as soon as IO removal is completed, and tests are not using mediated connection anymore (which will be possible with Remove msg-sending IO for issuer and holder #946 merged) we can get back to this, and at very least lift these *_find_messages_to_handle code-smells a level up to libvcx_core, where it can be (or not) dealt with separately.

[Patrik-Stas]

Thanks for reviews, everything has been addressed, please approve if all looks good