Added MutatedActiveLoan storage for PoV optimization

[lemunozm]

Description

Fixes #1193

Tasks

• Implementation
• Test passing
• Update benchmarks

Changes and Descriptions

• Added new storage MutatedActiveLoans.
• Remove not needed loan_id from ActiveLoan.

Type of change

• Breaking change

[lemunozm]

I think this change adds a lot of complexity to the code and makes the benchmarks difficult to measure correctly. Please double-check this. Is this PR really really needed if we get #1269 done?

[lemunozm]

A better name for this method?

[mustermeiszer]

push_fetch_(trying_to_at_least)

[lemunozm]

I do not get well what you mean here 🙂

[mustermeiszer]

I think this change adds a lot of complexity to the code and makes the benchmarks difficult to measure correctly. Please double-check this. Is this PR really really needed if we get #1269 done?

I agree. We should definitely wait till PoV is benched too.

And given a second thought, why would this even be a reduction in PoV size? It rather looks like an increase, but maybe I am getting the size impact not correctly

For a successful state transition from block x to block x+1 the relay chain needs

• block x+1 → extrinsics and final root
• (key, value)-pairs of touched keys at the state of x

if this assumption is correct, than we will anyways send the whole vector all the time, due to reading.

[lemunozm]

I think (or I understand from previous conversations) each extrinsic is measured isolated for the PoV Size.

So, for each active loan modification, we are writing now into MutatedActiveLoans storage which is smaller than ActiveLoans, so we are not increasing too much the PoV Size.

This increases the PoV Size for portfolio_valuation, but it's ok if other calls are smaller. Statistically, if we sum all extrinsic done in pallet-loans per block, the PoV Size will be smaller than writing always the whole ActiveLoan vector for each modification of each loan.

[mustermeiszer]

I think (or I understand from previous conversations) each extrinsic is measured isolated for the PoV Size.

Ahh, yeah, that is correct and a benchmarking issue... mmh

[mustermeiszer]

cc @lemunozm

[lemunozm]

I think we can close it 👍🏻. In case we want to go with this solution we can reopen, although I think the database has changed a lot from this.