Emit event when changing total locked value in pallet-balances

[sea212]

Fixes #12276

This PR adds two events to the balances pallet, namely:

/// Some free balance was locked out from specific operations.
Locked { who: T::AccountId, amount: T::Balance, reason: Reasons },
/// Some balance that was locked out from specific operations was freed.
Unlocked { who: T::AccountId, amount: T::Balance, reason: Reasons },

Since locks in the same category of Reasons overlay each other, the design decision was made to include only the changes of total lock value in the amount fields of the respective events. Examples:

Locks = [10/MISC]          --> [15/MISC]              // emits Locked(..., amount=5, Reasons::Misc)
Locks = [15/MISC]          --> [15/MISC, 20/MISC]     // emits Locked(..., amount=5, Reasons::Misc)
Locks = [15/MISC, 20/MISC] --> [17/MISC, 20/MISC]     // emits nothing
Locks = [17/MISC, 20/MISC] --> [20/MISC]              // emits nothing
Locks = [20/MISC]          --> [10/MISC]              // emits Unlocked(..., amount=10, Reasons::Misc)
Locks = [10/MISC]          --> [10/MISC, 15/FEE]      // emits Locked(..., amount=15, Reasons::Fee)
Locks = [10/MISC, 15/FEE]  --> [20/MISC, 20/FEE]      // emits Locked(..., amount=10, Reasons::Misc) and
                                                      // emits Locked(..., amount=5, Reasons::Fee)
// etc.

There are also alternative approaches that could be applied:

1. Add total_locked field to the respective events.
2. Emit lock event that only contains total_locked instead of the amount that was added to or removed from the total lock value.

Some questions in regards to the actual changes in this PR:

1. Currently Reasons::All is not used in the event. There is a corner case when a lock is set with Reasons::All that implies the same relative change of lock amounts for both Reasons::Misc and Reasons::Fee (all currently available reasons). In that case the event could emit Reasons::All. I decided against adding this because in terms of implementation effort this does not scale well at all. Instead, two distinct events are now emitted.
2. The PR contains this logic: if a > b then a - b and if a < b then b - a. Is it okay to use non-saturating math here or is using saturating math only enforced meanwhile?

[cla-bot-2021[bot]]

User @sea212, please sign the CLA here.

[ggwpez]

bot rebase

[paritytech-processbot[bot]]

Rebased

[ggwpez]

Emit lock event that only contains total_locked instead of the amount that was added to or removed from the total lock value.

I think both is fine. In both cases you either dont have the diff or not the total but putting both in the event could be done :man_shrugging:

[bkchr]

bot rebase

[paritytech-processbot[bot]]

Error: Command 'Command { std: "git" "push" "--porcelain" "sea212" "balances-add-lock-events", kill_on_drop: false }' failed with status Some(1); output: error: failed to push some refs to 'https://x-access-token:${SECRET}@github.com/sea212/substrate.git'

[bkchr]

@sea212 please merge master.

[sea212]

@sea212 please merge master.

@bkchr Rebased the branch onto main and squashed some commits together.

You can compare that only the commits from Substrate were rebased unmodified onto main and you can compare that the changes in this PR are equal pre and post rebase.

[bkchr]

@sea212 can you please fix the CI errors:

--- STDERR:              pallet-offences-benchmarking bench_report_offence_im_online ---
thread 'bench_report_offence_im_online' panicked at 'Mismatching events.', frame/offences/benchmarking/src/lib.rs:276:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/panicking.rs:65:14
   2: core::ops::function::FnOnce::call_once{{vtable.shim}}
   3: pallet_offences_benchmarking::Pallet<T>::test_benchmark_report_offence_babe::{{closure}}
   4: pallet_offences_benchmarking::Pallet<T>::test_benchmark_report_offence_im_online
   5: std::thread::local::LocalKey<T>::with
   6: core::ops::function::FnOnce::call_once
   7: core::ops::function::FnOnce::call_once
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/ops/function.rs:251:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
thread 'bench_report_offence_im_online' panicked at 'report_offence_im_online: Any { .. }', frame/offences/benchmarking/src/lib.rs:286:1
stack backtrace:

[sea212]

Since some big changes in pallet-balances were introduced on main, this PR probably needs a review again. Essentially everything stayed the same, but now the lock Reason is ignored in the Locked/Unlocked events. In addition to that, the tests were adjusted to ignore the lock Reason as well. Those conditions are tested now:

Locks = []        --> [10]      // emits Locked(..., amount=10)
Locks = [10]      --> [15]      // emits Locked(..., amount=5)
Locks = [15]      --> [15, 20]  // emits Locked(..., amount=5)
Locks = [15, 20]  --> [17, 20]  // emits nothing
Locks = [17, 20]  --> [17, 15]  // emits Unlocked(..., amount=3)
Locks = [17, 15]  --> [15]      // emits Unlocked(..., amount=2)
Locks = [15]      --> []        // emits Unlocked(..., amount=15)

For the sake of clarity and convenience to review, decided to create a second PR that handles the events for the new freeze and thaw calls that deprecate the locks.

[sea212]

Does anyone know what is going on here in regards to the CI? If I run the command locally all tests pass. The changes in this PR are even not related to the errors that occur.

@bkchr @kianenigma @ggwpez @melekes

[paritytech-processbot[bot]]

Error: Statuses failed for d01e57bfc99ddbed8c780397b346647c3a8a341c

[bkchr]

bot rebase

[paritytech-processbot[bot]]

Rebased

[bkchr]

bot merge

[paritytech-processbot[bot]]

Waiting for commit status.

[paritytech-processbot[bot]]

Merge cancelled due to error. Error: Statuses failed for 2e8c08d2a483c398bdd5374b6bec67da16250fb9

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs. Job name: test-linux-stable Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2589497

[ggwpez]

Master is red now, but there were internal CI problems. Will fix.