refactor/request: simplify MutateMDataEntries requests

[mrcnski]

No description provided.

[lionel-faber]

I'm good with these changes from an SCL standpoint. Vaults will require changes for this too.

[mrcnski]

The latest commit adds a new error in case of a type mismatch. This mismatch was impossible before this PR (we had two request types for seq and unseq separately, not one request for both), so I'm wondering if a slightly simplified API is worth the increased potential for errors. I'm leaning towards closing this PR.

[mrcnski]

I was wrong - the type mismatch was possible before this PR, because you could provide a Seq address to MutateUnseqMDataEntries, and vice versa. This change is just making a different type mismatch possible (between the provided address and new MDataEntries type) while simplifying the overall API. Reopening.

[madadam]

actions is passed by value so I don't think the clone is necessary.

[mrcnski]

If I try removing the clone I get this error:

error[E0507]: cannot move out of borrowed content
   --> src/mutable_data.rs:747:37
    |
747 |                 data.mutate_entries(*actions, requester)
    |                                     ^^^^^^^^ cannot move out of borrowed content

If I try not taking actions by reference I get this:

error[E0009]: cannot bind by-move and by-ref in the same pattern
   --> src/mutable_data.rs:749:53
    |
749 |             (Data::Unseq(data), EntryActions::Unseq(actions)) => {
    |                          ----                       ^^^^^^^ by-move pattern here
    |                          |
    |                          both by-ref and by-move used

Don't see a way around these errors, so I left the code as-is and added a comment.

[madadam]

Hm, just tried in in rust playground. Apparently it's not possible to match by reference and by move in the same pattern. A way around this could be to restructure it somehow like this:

match self {
    Data::Seq(data) => if let EntryActions::Seq(actions) = actions {
        return data.mutate_entries(actions, requester);
    },
    Data::Unseq(data) => if let EntryAction::Unseq(actions) = actions {
        return data.mutate_entries(actions, requester);
    }
}

Error(Err::InvalidOperation)

[madadam]

Can we avoid introducing this variant and use just InvalidOperation instead?