Calculate UpdatedAddresses on Lib9c side

[longfin]

Summary

This PR adds .CalculateUpdatedAddresses() to IEnumerable<PolymorphicAction<ActionBase>> and IEnumerable<ActionBase> to get updated addresses handy.

Background

Transaction<T>.UpdatedAddresses is no longer auto-calculated.

• This feature isn't necessary for Libplanet 1.0.* yet, because it will be calculated on Transaction<T>.Create().
• But also, Libplanet has planned to remove auto calculation since it isn't realistic to compute all through previous states to make a transaction, and complicates the implementation.
• see also:
  • Get rid of Transaction<T>.UpdatedAddresses and “rehearsal mode”, and more libplanet#368
  • Stop calculate Transaction<T>.UpdatedAddresses automatically libplanet#3122

Then, why do we still need it?

• However even now, 'UpdatedAddresses' are automatically written in transactions on 9c mainnet.
  • This is possible because most of the actions so far are writing compatible code assuming the simple state base based on the IActionContext.Rehearsal.
  • E.g.,

    lib9c/Lib9c/Action/TransferAsset.cs

    Lines 92 to 98 in 6912e58

    	public override IAccountStateDelta Execute(IActionContext context)
    	{
    	var state = context.PreviousStates;
    	if (context.Rehearsal)
    	{
    	return state.MarkBalanceChanged(Amount.Currency, new[] { Sender, Recipient });
    	}

• Sadly, related systems in 9c mainnet already seems to be relying Transaction<T>.UpdatedAddresses.
  • E.g., https://github.com/planetarium/9cscan-cloud/blob/0760e14f4ed07997d96784fcbc3841cd6790083f/project/sync/src/sync.js#L34-L40
• Thus, even if Libplanet decides to make the block layout compatible, the related system compatibility may be broken against empty UpdatedAddresses.
• As one of Libplanet devs, I really want to (re)organize tx/action evaluation flow without the auto-filling assumption of updated addresses.
• but also as one of 9c devs, I don't want to face an ironic situation like, updating Libplanet (having "good design") kills related services in the ecosystem.

Plan (9c side only)

• Add methods to calculate .UpdatedAddresses leveraging existing Rehearsal related codes to Lib9c (this PR)
• Add updatedAddresses argument explicitly for Transaction<T>.Create() when creating transactions (NineChronicles / NineChronicles.Headless).
• (Optional) Provide a migration path from relying on Transaction<T>.UpdatedAddresses to using ActionEvaluation.UpdatedAddresses for systems involving 9c mainnet.

[moreal]

• Sadly, related systems in 9c mainnet already seems to be relying Transaction.UpdatedAddresses.
  • E.g., https://github.com/planetarium/9cscan-cloud/blob/0760e14f4ed07997d96784fcbc3841cd6790083f/project/sync/src/sync.js#L34-L40

I think it is not an issue related to Transaction<T>.UpdatedAddresses. If they want to know what states are updated by actions, they should use TxSuccess.UpdatedStates but they and we don't want to store the updated states too so they cannot use them. I think that is the real problem.

Now, even the 9c-main-full-state node, they don't store the TxSuccess.UpdatedStates. If you want to query them, you can use my node. https://9c-api.moreal.dev/ui/playground

https://9cscan.com/tx/3b81c8e5af404eb7d4ef5e2e462bc27306440b7c9ae164d8eee1b6193242f286

query {
  transaction {
    transactionResult(txId: "3b81c8e5af404eb7d4ef5e2e462bc27306440b7c9ae164d8eee1b6193242f286") {
      updatedStates {
        address
        state
      }
    }
  }
}

[moreal]

At the code level, I think it can be merged.

[longfin]

I think it is not an issue related to Transaction<T>.UpdatedAddresses. If they want to know what states are updated by actions, they should use TxSuccess.UpdatedStates but they and we don't want to store the updated states too so they cannot use them. I think that is the real problem.

It's true. but the point I wanted to mention is, many systems are already using Transaction<T>.UpdatedAddresses with assumptions based on the previous behavior (i.e., autofill with dry-run). in this situation, planetarium/libplanet#3122 would be a breaking change if we don't have any other polyfill (like this PR), to mimic.

But on the other hand, I believe that we need to introduce a legit way to access "updated addresses", like as you mentioned. 😅

[pull-request-quantifier-deprecated]

This PR has 93 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

---

Quantification details

Label      : Small
Size       : +93 -0
Percentile : 37.2%

Total files changed: 2

Change summary by file extension:
.cs : +93 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

• Fast and predictable releases to production:
  • Optimal size changes are more likely to be reviewed faster with fewer
    iterations.
  • Similarity in low PR complexity drives similar review times.
• Review quality is likely higher as complexity is lower:
  • Bugs are more likely to be detected.
  • Code inconsistencies are more likely to be detected.
• Knowledge sharing is improved within the participants:
  • Small portions can be assimilated better.
• Better engineering practices are exercised:
  • Solving big problems by dividing them in well contained, smaller problems.
  • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

• Use the PullRequestQuantifier to quantify your PR accurately
  • Create a context profile for your repo using the context generator
  • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
  • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
  • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
• Change your engineering behaviors
  • For PRs that fall outside of the desired spectrum, review the details and check if:
    • Your PR could be split in smaller, self-contained PRs instead
    • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

• One line was added: +1 -0
• One line was deleted: +0 -1
• One line was modified: +1 -1 (git diff doesn't know about modified, it will
  interpret that line like one addition plus one deletion)
• Change percentiles: Change characteristics (addition, deletion, modification)
  of this PR in relation to all other PRs within the repository.

---

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

[longfin]

Just for note) At least in v200010, many of the actions in Lib9c already seem to stop filling UpdatedAddresses using IActionContext.Rehearsal. 😅 (e.g., HAS20)