Refactor messages/agency_comm module, delete legacy proof

[Patrik-Stas]

Bunch of cleanusp and refactors packed here:

• Deleted legacy proof code (code around ProofRequestMessage)

• Renamed messages to agency_comm to clearly signal the module is dealing with communication with agency

• Took src/disclosed_proof_utils.rs and src/proof_utils and along some still-used-datastructures which was located along with legacy proofs src/messages/proofs/*.rs restructured them into newly created module libindy (see src/libindy/**).

• Also moved utils/libindy to libindy/utils. I think all libindy wrapping code should be in one place to be able to modularize credentials/ledgers and possibly swap implementations in the future. So this would be just a beginning, there still fragments of libindy wrapping code in utils/ and other part of codebase

• Started breaking up global settings - I've created src/agency_comm/agency_settings.rs which looks a lot like the src/settings.rs, but only deals with CONFIG_AGENCY_ENDPOINT, CONFIG_AGENCY_DID, CONFIG_AGENCY_VERKEY, CONFIG_REMOTE_TO_SDK_DID, CONFIG_REMOTE_TO_SDK_VERKEY, CONFIG_SDK_TO_REMOTE_DID, CONFIG_SDK_TO_REMOTE_VERKEY.

  Removing dependency on ::settings us closer toward making agency_comm standalone module which could be used without using the rest of aries-vcx.

  Naturally I've removed these from the src/settings.rs. To keep things working from user perspective, some calls into src/settings.rsare propagated further to src/agency_comm/agency_settings.rs such as fn validate_config, fn set_testing_defaults, fn clear_config, fn process_config_string.

[codecov-io]

Codecov Report

Merging #149 into master will decrease coverage by 0.06%.
The diff coverage is 58.20%.

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
- Coverage   50.86%   50.79%   -0.07%     
==========================================
  Files         151      156       +5     
  Lines       23247    22918     -329     
  Branches     6138     6091      -47     
==========================================
- Hits        11825    11642     -183     
+ Misses       7754     7647     -107     
+ Partials     3668     3629      -39     

Flag	Coverage Δ
unittests	50.79% <58.20%> (-0.07%)	⬇️

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

Impacted Files	Coverage Δ
libvcx/src/agency_comm/payload.rs	0.00% <ø> (ø)
libvcx/src/agency_comm/thread.rs	71.87% <ø> (ø)
libvcx/src/agency_comm/update_message.rs	27.77% <0.00%> (ø)
libvcx/src/api/credential.rs	33.56% <ø> (ø)
libvcx/src/api/credential_def.rs	51.12% <ø> (-0.20%)	⬇️
libvcx/src/api/disclosed_proof.rs	41.04% <0.00%> (-1.34%)	⬇️
libvcx/src/api/issuer_credential.rs	36.88% <ø> (ø)
libvcx/src/api/return_types_u32.rs	64.06% <ø> (ø)
libvcx/src/api/schema.rs	56.32% <ø> (-0.25%)	⬇️
libvcx/src/api/wallet.rs	49.76% <ø> (ø)
... and 87 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49e7732...cef5209. Read the comment docs.

[mirgee]

I think agency_vcx is way more confusing and inaccurate than messages. How about something like comm? Or, if you want to mention agency, than something like agency_comm or agency_messages... This is just throwing ideas out there.

[mirgee]

I think it would be preferrable to have all config-related constants on one place (although splitting settings.rs is not a bad idea), unless they were used only in this module

[Patrik-Stas]

Though I disagree about placement to settings.rs as the idea is to make agency_comm standalone, thanks to your comment I've noticed that these things actually do not need to be static at all. These are only used within set_testing_defaults_agency function in agency_settings.rs, so I am moving it right there.

[mirgee]

I didn't say anything about putting to settings.rs, by one place I mean e.g. two submodules of a single module. I see your point about benefits of making agency-related logic standalone, but these configs are going to be used throughout libvcx even in case agency_comm were to become separate crate, they are part of the interface between libvcx and agency_comm, and thus are global to the app in that sense.

[Patrik-Stas]

Well, these particular here are default values used in agency tests, and as I've found, really just local to 1 single test.

[mirgee]

By "config-related constants" I meant the config keys as well. But if the plan is to get rid of agency_settings.rs anyways (which I agree with), let's keep it as it is for now.

[mirgee]

I wonder why is skip_serializing_if necessary?

[Patrik-Stas]

Well, I haven't done modifications here, just moved the datastructure, however I suppose it makes sense to skip serializing the field if there's no value set.

[mirgee]

But why is it necessary? Extra fields are ignored during deserialization...

[Patrik-Stas]

Not ignored

use serde_json;

#[derive(Serialize)]
struct Data {
    #[serde(skip_serializing_if = "Option::is_none")]
    b: Option<String>,
    c: Option<String>,
}

pub fn run()
{
    let data = Data { b: None, c:None};
    let serialized = serde_json::to_string(&data).unwrap();
    println!("serialized={}", serialized);
}

serialized={"c":null}

[mirgee]

I said "deserialization", not "serialization" :) I was just wondering if there is any actual functional reason why these specific field must be skipped if none, but I guess this is just a sporadic attempt to reduce the payload size...

[mirgee]

One of the reasons all settings should share a module, IMO

[Patrik-Stas]

Disagree, this "redirection" is to simply preserve aries-vcx functionality as a whole, but removing dependencies on aries-vcx globals is precondition to create independent modules.

[mirgee]

What do you mean by redirection? I think that on the contrary, libvcx, as a "user" of agency_comm, has the responsibility of configuring it as necessary. As I see it, agency_settingsis a subset of libvcx settings, not separate from it.

[Patrik-Stas]

By redirection I meant that call into the src/settings.rs further calls agency_settings.rs.

But yeah, it could be removed and code in all places would have to make calls into agency_settings.rs manually when needed. It was more convenient to implement this way, with "the redirection", but I also don't see this as ultimate state. I think we could eliminate agency_settings.rs completely in the next steps.