SDK Changes required to support Rosetta balance tracking

[fdymylja]

We've received feedback from coinbase engineers in the rosetta forum, they require us to support full balance tracking via rosetta API.

Balance tracking means that each balance change must be represented as a set of operations (1+ operations are encapsulated into a TX) as inputs and outputs. And from this set of inputs and outputs the balance computed must match what is returned when querying the account balance.

Example:

{
   "tx":{
      "amounts":[
         {
            "address":"cosmos1",
            "coinChange":"-1ATOM"
         },
         {
            "address":"cosmos2",
            "coinChange":"+1ATOM"
         }
      ]
   }
}

One problem arises, balance changes can happen during endblock and beginblock, and we have no way to query for changes that happened in those phases. Since EndBlock and BeginBlock state changes are not represented via transactions.

So this forces us to parse balance changes via events.

In order to do so in a consistent way, we need to do some changes to the bank keeper, in particular the methods that take care of handling supply.

The bank changes would be the following:

• Encapsulate the core supply logic of bank into three functions: SendCoins, MintCoins and BurnCoins. These methods will be the core of supply tracking mechanism, each action that will change supply somehow will have to end up into these three methods which will spawn an event related to the supply changes.
• Limit the methods that bank exposes: the low level functions 'SetBalance', 'AddCoins', should not be exposed, otherwise they can be abused to bypass balance tracking. After this API change, some modules might need to be changed to use the SendCoins/Burn/Mint methods
• Fix BankKeeper utility methods such as Undelegate which use low level functions such as SetBalance which bypass balance tracking events.

Aside from changes to the bank module, some changes might be required for the events, since rosetta needs to know which transaction hash has caused the balance change, which means we will be forced to forge begin and endblock transaction hashes (only in the rosetta implementation, this is not an sdk change), we will need to know which event happened at which stage (beginblock, tx1, tx2, endblock).

We have not tested if tendermint events are in someway linkable to Begin/Endblock or to which transaction in the block.

[fdymylja]

After some research, bank needs to be changed in order not to expose methods that can bypass supply tracking, no changes on the events are required as of now.

Although I must say that having the intermodule ADR implemented would allow us to extrapolate hidden module calls (example: MsgStake hides a bank send) at DeliverTx, BeginBlock and EndBlock stages.

If all the calls between modules were processed as messages then we could generate events at the handler levels to highlight what module calls each message is doing.

There's still the overlapping event bug highlighted in this issue which is currently a blocker for correct balance tracking. Ref: #8419

[aaronc]

@fdymylja these are pretty similar to the concerns I raised in #7093 and that motivated #7459. I started working on a refactor here to address all of this: regen-network/regen-ledger#203. It's basically the same as what you're saying - Send, Mint and Burn should basically be the only exposed methods and there should never be any way for other modules to bypass supply tracking or events. I had been planning to present this in an architecture call soon - maybe we can do it this coming Friday.

[jgimeno]

Hi @aaronc ! The idea is to release the second part of the Rosetta integration to support full balance tracking as soon as possible. I would sugest to arrive to a compromise in order not to depend on the intermodule for releasing rosetta. Is that possible? Thanks!

[aaronc]

I would just note that the ADR 033 implementation is actually done on our end, we just don't have the ADR actually approved on the SDK side. So I wouldn't consider that blocking necessarily... The bigger issue I think is actually refactoring the bank module and other modules to do this properly.

[fdymylja]

I have inspected the modules using unsafe methods, and currently the following use them:

• gov
• distribution
• staking

They all use SetBalance at init genesis level, I still haven't been able to understand why those modules would need to set balance specifically at this stage, why doesn't bank already have this information?

Since probably I'm missing something, could a SetBalance call be substituted by a mint or mint&send combo?