Proto signing fails when accountNumber = 0

[jordaaash]

All the tests for proto signing use 1 as the accountNumber:

cosmjs/packages/proto-signing/src/signing.spec.ts

Line 94 in c0b714b

const accountNumber = 1;

cosmjs/packages/proto-signing/src/directsecp256k1hdwallet.spec.ts

Line 71 in c0b714b

const accountNumber = 1;

cosmjs/packages/proto-signing/src/directsecp256k1wallet.spec.ts

Line 47 in c0b714b

const accountNumber = 1;

We had a situation where we tried to send tokens from accountNumber: 0, which gets converted to a Long of {low: 0, high: null, unsigned: null} here:

cosmjs/packages/proto-signing/src/signing.ts

Line 44 in c0b714b

accountNumber: Long.fromNumber(accountNumber),

omitDefaults then recursively converts this to a plain JS object of {low: null, high: null, unsigned: null} here:

cosmjs/packages/proto-signing/src/adr27.ts

Line 43 in c0b714b

if (isNonNullObject(input)) {

This leads to an invalid signature (ref: ignite/web#39)

@webmaster128 @jgimeno @vince19972 @fadeev

[webmaster128]

Nice catch! Thank you for the detailed report. In #550 you find a fix for this.

Testing this higher level is not trivial but possible. It would require us to use the private key for the account number 0 (i.e. the validator account), which we don't have a mnemonic for. However, in this case the bug is well understood, so it should work with the PR merged.

[webmaster128]

The fix was released yesterday as part of 0.24.0-alpha.10

[clockworkgr]

This seems to have crept up again, I'm not entirely sure what's going on but it was fixed...then with the switch to ts-proto it broke again but I'm guessing at 122bb2b (which is not published yet) it will be fixed again? (Judging by the fact the isZero() checks seem to have been added there)

@webmaster128 Can you confirm this is the case? (if so, when are you publishing next ? :D)

[webmaster128]

Right. With the ts-proto migration PR the adr27 module containing the fix got unused, which is a bug. It was fixed with a ts-proto update, which omits all default values.

I'll publish an update by the end of the day.

[clockworkgr]

Awesome!

[webmaster128]

0.24.0-alpha.26 is released, which should fix this again since we regenerated internal types with a version of ts-proto that does not serialize default values like accountNumber = 0.

[angelorc]

At the moment I'm using 0.26.0 version and it seems that the problem has returned.

I launched a new chain with 2 accounts (accounts 0 and 1), if I try to make a transaction with account 0 I always receive the incorrect signature error. If I try to carry out the same operations with Account 1 I'm not getting any errors.

I performed the same test from Command Line and all two accounts work properly.

Do you have any idea on how can I solve this?

[webmaster128]

@angelorc could you open a new ticket for you issue to be able to track it? It's probably a different issue. Could you also include Cosmos SDK version, sign mode, @cosmjs/ package you use, wallet type and message type?

[angelorc]

@webmaster128 done #961