multi-msg batch txs need msg index shown in raw_log for usability

[jaekwon]

I tried to submit a batch tx with many messages for redelegating.

The error I get is:

...:~/go/src/github.com/atomone$ ./atomoned-v1.0.0-linux-amd64 tx broadcast signed.txt --node https://atomone-rpc.allinbits.services:443
code: 18
codespace: sdk
data: ""
events: []
gas_used: "0"
gas_wanted: "0"
height: "0"
info: ""
logs: []
raw_log: 'invalid shares amount: invalid request'
timestamp: ""
tx: null
txhash: E8DD1718BA7B54B876F31C0182358E886EDE9F68662FB98564EBE47ADDDB3729 

But this isn't helpful if there are many transactions! The raw_log should include the index of the msg that error'd.

[jaekwon]

There were a few bad msgs in the batch tx.

Some of these msgs had amounts that exceeded the available atone.
But the thing that triggered the broadcast to fail seems to have been a msg that tried to redelegate 0 atone.

When I tried to resubmit with fixes, I ended up getting an error not in the broadcast, but after successful tx submission. The transaction failed with: failed to execute message; message index: 17: invalid shares amount: invalid request

https://www.mintscan.io/atomone/tx/6D9D224AA4E6624B070834B74402C902FC6B09DC3854D9D7F4DA1F13E85FDC3C?height=5933301

>>> 26164923976 + 90303010872 + 89188065151
205655999999 <--- total uatone redelegation
>>> 205656000000
205656000000 <--- actual uatone available for redelegation

This was due to a rounding issue between uatone and shares, obviously.

Q1: Why was the index not shown in broadcast failure but was shown in mintscan?
Q2: Is there any way we can make this rounding issue less of a problem?

[tbruyelle]

@jaekwon

Q1: Why was the index not shown in broadcast failure but was shown in mintscan?

Because the SDK doesn't prepend the index for errors returned by msg.ValidateBasic().

Now the reason why a better error is visible on Mintscan is because it's not the same error. The first error was because amount=0, which is detected by ValidateBasic, so during checkTx, while the second error happens later during message execution (checkTx has passed), and here the message index is correctly prepend to the error.

To fix this for v3 we have to fork 0.47 (only AtomOne v4 uses our fork).

The problem does not happen in v4 though, because most msg.ValidateBasics were removed in SDK50. So the same invalid tx will be included in a block as an erronous tx, and the fees will be charged (like for your second attempt when you resubmitted with fixes).

I'm trying to understand what's the benefit of making msg.ValidateBasic optionnal, because at first glance it just looks like a way to charge more fees than before.

Q2: Is there any way we can make this rounding issue less of a problem?

I helped Albert fix the multi-delegation tx on the testnet, and we encountered this invalid shares amount error.

This was because the amount was taken from the shares field of the delegation, instead of the amount field. These fields are equals only if the delegation was never slashed.

Example of slashed delegation:

- balance:
    amount: "99990000"
    denom: uatone
  delegation:
    delegator_address: atone174cpmh5ddhljeaclas3x3nkr277jajtfa8z5h5
    shares: "100000000.000000000000000000"
    validator_address: atonevaloper14pvura4yw3pf2ceppg2kzlqxsjpdwvtfzkumyj

So I don't think it's a rounding issue, it's because the script used to build the tx was using the shares field instead of the amount field to feed the amount field in the redelegate msg.