[R4R] fix account may have currency with zero balance

[unclezoro]

Description

resolve issue #677

Rationale

Some consideration for the fixing:

1. Why not change Plus function for Coins.
   May need check every currency value of the coin, may hurt the performance.
2. Is there any point that call Plus may have such issue?
   Swap, Freeze, Timelock, Send, Mint, Burn , Issue have checked the coins to be positive, this only happend when add an currency which is possiable to be zero, for now only trade will involved.

Example

add an example CLI or API response...

Changes

Notable changes:

• add each change in a bullet point here
• ...

Preflight checks

• build passed (make build)
• tests passed (make test)
• integration tests passed (make integration_test)
• manual transaction test passed (cli invoke)

Already reviewed by

...

Related issues

#667

[ackratos]

if remain := tran.unlock-tran.out; remain > 0 || !sdk.IsUpgrade... {
     accountCoin = accountCoin.Plus(sdk.Coins{sdk.NewCoin(tran.outAsset, remain)})
}

[unclezoro]

not sure whether we want clean up historical data at a specific height......

Amazing, this is better in performance.

[HaoyangLiu]

Why not this:

if tran.unlock>tran.out || !sdk.IsUpgrade... {

[ackratos]

not sure whether we want clean up historical data at a specific height......

[unclezoro]

not sure whether we want clean up historical data at a specific height......

emm, this may stop the world for a while, I am not sure either. @yutianwu @HaoyangLiu @darren-liu @rickyyangz any suggest?

[yutianwu]

it seems acceptable if we clean the accounts in prod for there are only 300k accounts in prod. and there are 700k in testnet. we can test how long will it take to clean the accounts.

[rickyyangz]

we need to find out how many accounts that have such case to estimate the cost. I think there should not be too many and then it may be acceptable to handle it in one height.

[unclezoro]

I will sync the prod data and try how many time it takes.

[unclezoro]

@rickyyangz @ackratos @HaoyangLiu @yutianwu Please see the test and statistics info in #684. The time may be acceptable, but during writing the algorithm that deletes the coin in coin slice, I really afraid of deleting the wrong coin which may totally ruin our chain. I don't want to take such risk, especially there is no strong requirement for this. Please comment.

[HaoyangLiu]

I prefer just leave them alone. Doing this has no obvious gain but take big risk.

[yutianwu]

ok, just let it go, and record this issue just in case we need to update the accounts.

or we can just airdrop some tokens to addresses affected, lol

[ackratos]

@rickyyangz @ackratos @HaoyangLiu @yutianwu Please see the test and statistics info in #684. The time may be acceptable, but during writing the algorithm that deletes the coin in coin slice, I really afraid of deleting the wrong coin which may totally ruin our chain. I don't want to take such risk, especially there is no strong requirement for this. Please comment.

agree