Closed greymistcube closed 1 month ago
@OnedgeLee
Do this PR contains state migration test? Or, existing test can cover currency migration?
Migration is mostly covered by ActionEvaluatorTest.Migration.cs
. Although now that I look at it, I should add Currency
manipulation to the action used in MigrationThroughBlock
test. π
@OnedgeLee
I've added an additional test regarding migration. See 4e5fc2fd6d08c8798f3e71dfdbe28b9eb8e93c29.
The implementation is pretty convoluted at the moment. π There are numerous issues regarding
Currency
:Currency
.Currency
manipulation pipeline is running on top of the assumption that a state is "consistent"Currency
:TotalSupply
does not exceedMaximumSupply
TotalSupply
is being tracked and is correct.TotalSupplyTrackable
loses its meaning starting withBlockMetadata.CurrencyAccountProtocolVersion
.Exception
forTotalSupply
.Exception
regardingTotalSupply
must be handled fromIAction
side (or any outside service), and its interpretation requires additional knowledge aboutCurrency
and the behavior of read operation ofTotalSupply
.MintAsset()
orBurnAsset()
should be preceded by authority validation.IAction
s.This is partially due to information about
Currency
not being recorded in a state and migration toCurrencyAccount
model does not permit adding in this missing information.I'll be making some
Currency
spec and API changes in separate PRs to mitigate some of these issues. πΆ