[gnokey] Verify and Sign function are not harmonious

[thinhnx-var]

Description

Start from this disscusion, I dive into gnokey command and find out somethings that weird with Sign() and Verify()
cc: @zivkovicmilos @leohhhn
Definition:
Verify: To verify a transaction signature is correct
Sign: To Sign the document.

Lets say we save our transaction into file.tx.

gnokey maketx call -pkgpath "gno.land/r/thinhnx/render" -func "Hello" -gas-fee 1000000ugnot -gas-wanted 2000000 myKey > file.tx

Then sign it with:

gnokey sign -tx-path file.tx -chainid "dev" -account-number XX -account-sequence YY myKey

There is a tx.GetSignBytes() that is used inside Sign() gathers arguments from command, in the signOptions:

signBytes, err := tx.GetSignBytes(
  signOpts.chainID,
  signOpts.accountNumber,
  signOpts.accountSequence,
)

Then adds some informations of tx: tx.Fee, tx.Msgs, tx.Memo (from the file.tx) to contruct a SignDoc data.
This SignDoc data will be sorted and marshaled to a []byte signBytes message.
The sign() function will take this final message as a message to sign.

Then the sign() signs this msg, with the corresponse keys, and then writes it (append) into the originial file.tx.

Problems:

The problem is here. The Verify() now take the whole file.tx after signed as input to verify. So, there are things that we should make it clearer: (which I think currently not work)

1. This Verify() will not works as if its input is the whole file after signed because file.tx is already changed and different from the signBytes. AFAIK we should have same message hash for sign and verify (?)

2. Since it is an oneway function, and the additional information which is added into the tx.GetSignBytes() comes from command, how to reconstruct the original signBytes to verify?

3. If we not need to / can not recontruct the orignal signBytes, then should we expose the signedMessage ?

4. Where can user take the hex format to feed the Verify command. And since the Sign() writes with signature (JSON encoded) into the file.tx, can we remove this argument?

WIP:

I am currently working around this and something relates. I am really looking to hearing from all of you.

[thinhnx-var]

I changed the verify flow into this with msg, _ := os.ReadFile(path)
and

afterSignedMsg := std.Tx{}

amino.UnmarshalJSON(msg, &afterSignedMsg)

//Test
theOrigMessageHash, err := afterSignedMsg.GetSignBytes("dev", 56, 0)
err = kb.Verify(name, theOrigMessageHash, sig)

the Verify() is valid.

Im now confusing another way to get the originalMessageHash instead of read the whole file.tx which is not true.

[thinhnx-var]

I can reconstruct the messageHash with chainID, account-number and account-sequence.
Should I query for it or just take it as arguments of Verify command?
@zivkovicmilos.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Remove stale label or comment or this will be closed in 3 months.