Skip to content

Commit

Permalink
fix: remove empty timestamp from the tx signature payload (gnolang#1939)
Browse files Browse the repository at this point in the history
## Description

This PR closes gnolang#810 

I've dropped the unused `Timestamp` field from the `SignDoc`, which
forced all clients that generate tx signatures outside of the `gno` repo
to initialize it to an empty UTC timestamp value (ex. Adena,
`tm2-js-client`...)

**BREAKING CHANGE**: all clients that generate transaction signatures
without using the provided Go methods in this repository need to update
their source code to remove the `Timestamp` field. (@dongwon8247
@jefft0)

I've also provided error handling for generating these signature bytes,
which weren't present before.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
zivkovicmilos committed Apr 29, 2024
1 parent 7bf662a commit 3832b13
Show file tree
Hide file tree
Showing 10 changed files with 228 additions and 99 deletions.
5 changes: 4 additions & 1 deletion gno.land/pkg/gnoclient/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ func (s SignerFromKeybase) Sign(cfg SignCfg) (*std.Tx, error) {
}

// Derive sign doc bytes.
signbz := tx.GetSignBytes(chainID, accountNumber, sequenceNumber)
signbz, err := tx.GetSignBytes(chainID, accountNumber, sequenceNumber)
if err != nil {
return nil, fmt.Errorf("unable to get tx signature payload, %w", err)
}

sig, pub, err := s.Keybase.Sign(account, password, signbz)
if err != nil {
Expand Down
16 changes: 11 additions & 5 deletions tm2/pkg/crypto/keys/client/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,21 @@ func signTx(
signOpts signOpts,
keyOpts keyOpts,
) error {
signBytes, err := tx.GetSignBytes(
signOpts.chainID,
signOpts.accountNumber,
signOpts.accountSequence,
)
if err != nil {
return fmt.Errorf("unable to get signature bytes, %w", err)
}

// Sign the transaction data
sig, pub, err := kb.Sign(
keyOpts.keyName,
keyOpts.decryptPass,
tx.GetSignBytes(
signOpts.chainID,
signOpts.accountNumber,
signOpts.accountSequence,
))
signBytes,
)
if err != nil {
return fmt.Errorf("unable to sign transaction bytes, %w", err)
}
Expand Down
10 changes: 8 additions & 2 deletions tm2/pkg/crypto/keys/client/sign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,10 @@ func TestSign_SignTx(t *testing.T) {
require.NoError(t, err)

// Generate the signature
signature, pubKey, err := kb.Sign(anotherKey, encryptPassword, tx.GetSignBytes("id", 1, 0))
signBytes, err := tx.GetSignBytes("id", 1, 0)
require.NoError(t, err)

signature, pubKey, err := kb.Sign(anotherKey, encryptPassword, signBytes)
require.NoError(t, err)

tx.Signatures = []std.Signature{
Expand Down Expand Up @@ -554,7 +557,10 @@ func TestSign_SignTx(t *testing.T) {
require.NoError(t, err)

// Generate the signature
signature, pubKey, err := kb.Sign(keyName, encryptPassword, tx.GetSignBytes("id", 0, 0))
signBytes, err := tx.GetSignBytes("id", 0, 0)
require.NoError(t, err)

signature, pubKey, err := kb.Sign(keyName, encryptPassword, signBytes)
require.NoError(t, err)

tx.Signatures = []std.Signature{
Expand Down
21 changes: 16 additions & 5 deletions tm2/pkg/sdk/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,11 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature
// No signatures are needed for genesis.
} else {
// Check signature
signBytes := GetSignBytes(newCtx.ChainID(), tx, sacc, isGenesis)
signBytes, err := GetSignBytes(newCtx.ChainID(), tx, sacc, isGenesis)
if err != nil {
return newCtx, res, true
}

signerAccs[i], res = processSig(newCtx, sacc, stdSigs[i], signBytes, simulate, params, sigGasConsumer)
if !res.IsOK() {
return newCtx, res, true
Expand Down Expand Up @@ -430,15 +434,22 @@ func SetGasMeter(simulate bool, ctx sdk.Context, gasLimit int64) sdk.Context {

// GetSignBytes returns a slice of bytes to sign over for a given transaction
// and an account.
func GetSignBytes(chainID string, tx std.Tx, acc std.Account, genesis bool) []byte {
func GetSignBytes(chainID string, tx std.Tx, acc std.Account, genesis bool) ([]byte, error) {
var accNum uint64
if !genesis {
accNum = acc.GetAccountNumber()
}
signbz := std.SignBytes(
chainID, accNum, acc.GetSequence(), tx.Fee, tx.Msgs, tx.Memo,

return std.GetSignaturePayload(
std.SignDoc{
ChainID: chainID,
AccountNumber: accNum,
Sequence: acc.GetSequence(),
Fee: tx.Fee,
Msgs: tx.Msgs,
Memo: tx.Memo,
},
)
return signbz
}

func abciResult(err error) sdk.Result {
Expand Down
Loading

0 comments on commit 3832b13

Please sign in to comment.