feat: add gas estimation to the signer (backport #3017) (#3144)

This is an automatic backport of pull request #3017 done by
[Mergify](https://mergify.com).
Cherry-pick of 2b13533 has failed:
```
On branch mergify/bp/v1.x/pr-3017
Your branch is up to date with 'origin/v1.x'.

You are currently cherry-picking commit 2b13533.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pkg/proof/proof_test.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pkg/user/signer.go
	both modified:   pkg/user/signer_test.go

```


To fix up this pull request, you can check it out locally. See
documentation:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the
[documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on
`<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you
can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

---------

Co-authored-by: Callum Waters <[email protected]>

pkg/user/signer.go

@@ -168,7 +168,7 @@ func (s *Signer) CreateTx(msgs []sdktypes.Msg, opts ...TxOption) ([]byte, error)
 		return nil, err
 	}
 
-	if err := s.signTransaction(txBuilder); err != nil {
+	if err := s.signTransaction(txBuilder, s.getAndIncrementSequence()); err != nil {
 		return nil, err
 	}
 
@@ -241,6 +241,31 @@ func (s *Signer) ConfirmTx(ctx context.Context, txHash string) (*sdktypes.TxResp
 	}
 }
 
+func (s *Signer) EstimateGas(ctx context.Context, msgs []sdktypes.Msg, opts ...TxOption) (uint64, error) {
+	txBuilder := s.txBuilder(opts...)
+	if err := txBuilder.SetMsgs(msgs...); err != nil {
+		return 0, err
+	}
+
+	if err := s.signTransaction(txBuilder, s.Sequence()); err != nil {
+		return 0, err
+	}
+
+	txBytes, err := s.enc.TxEncoder()(txBuilder.GetTx())
+	if err != nil {
+		return 0, err
+	}
+
+	resp, err := tx.NewServiceClient(s.grpc).Simulate(ctx, &tx.SimulateRequest{
+		TxBytes: txBytes,
+	})
+	if err != nil {
+		return 0, err
+	}
+
+	return resp.GasInfo.GasUsed, nil
+}
+
 // ChainID returns the chain ID of the signer.
 func (s *Signer) ChainID() string {
 	return s.chainID
@@ -268,8 +293,19 @@ func (s *Signer) PubKey() cryptotypes.PubKey {
 	return s.pk
 }
 
-// GetSequencer gets the lastest signed sequnce and increments the local sequence number
+func (s *Signer) Sequence() uint64 {
+	s.mtx.Lock()
+	defer s.mtx.Unlock()
+	return s.lastSignedSequence
+}
+
+// DEPRECATED: use Sequence instead
 func (s *Signer) GetSequence() uint64 {
+	return s.getAndIncrementSequence()
+}
+
+// getAndIncrementSequence gets the lastest signed sequnce and increments the local sequence number
+func (s *Signer) getAndIncrementSequence() uint64 {
 	s.mtx.Lock()
 	defer s.mtx.Unlock()
 	defer func() { s.lastSignedSequence++ }()
@@ -285,7 +321,12 @@ func (s *Signer) ForceSetSequence(seq uint64) {
 	s.lastSignedSequence = seq
 }
 
-func (s *Signer) signTransaction(builder client.TxBuilder) error {
+// Keyring exposes the signers underlying keyring
+func (s *Signer) Keyring() keyring.Keyring {
+	return s.keys
+}
+
+func (s *Signer) signTransaction(builder client.TxBuilder, sequence uint64) error {
 	signers := builder.GetTx().GetSigners()
 	if len(signers) != 1 {
 		return fmt.Errorf("expected 1 signer, got %d", len(signers))
@@ -295,20 +336,9 @@ func (s *Signer) signTransaction(builder client.TxBuilder) error {
 		return fmt.Errorf("expected signer %s, got %s", s.address.String(), signers[0].String())
 	}
 
-	sequence := s.GetSequence()
-
 	// To ensure we have the correct bytes to sign over we produce
 	// a dry run of the signing data
-	draftsigV2 := signing.SignatureV2{
-		PubKey: s.pk,
-		Data: &signing.SingleSignatureData{
-			SignMode:  signing.SignMode_SIGN_MODE_DIRECT,
-			Signature: nil,
-		},
-		Sequence: sequence,
-	}
-
-	err := builder.SetSignatures(draftsigV2)
+	err := builder.SetSignatures(s.getSignatureV2(sequence, nil))
 	if err != nil {
 		return fmt.Errorf("error setting draft signatures: %w", err)
 	}
@@ -318,16 +348,8 @@ func (s *Signer) signTransaction(builder client.TxBuilder) error {
 	if err != nil {
 		return fmt.Errorf("error creating signature: %w", err)
 	}
-	sigV2 := signing.SignatureV2{
-		PubKey: s.pk,
-		Data: &signing.SingleSignatureData{
-			SignMode:  signing.SignMode_SIGN_MODE_DIRECT,
-			Signature: signature,
-		},
-		Sequence: sequence,
-	}
 
-	err = builder.SetSignatures(sigV2)
+	err = builder.SetSignatures(s.getSignatureV2(sequence, signature))
 	if err != nil {
 		return fmt.Errorf("error setting signatures: %w", err)
 	}
@@ -390,3 +412,17 @@ func QueryAccount(ctx context.Context, conn *grpc.ClientConn, encCfg encoding.Co
 	accNum, seqNum = acc.GetAccountNumber(), acc.GetSequence()
 	return accNum, seqNum, nil
 }
+
+func (s *Signer) getSignatureV2(sequence uint64, signature []byte) signing.SignatureV2 {
+	sigV2 := signing.SignatureV2{
+		Data: &signing.SingleSignatureData{
+			SignMode:  signing.SignMode_SIGN_MODE_DIRECT,
+			Signature: signature,
+		},
+		Sequence: sequence,
+	}
+	if sequence == 0 {
+		sigV2.PubKey = s.pk
+	}
+	return sigV2
+}

pkg/user/signer_test.go

@@ -2,6 +2,7 @@ package user_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -13,8 +14,10 @@ import (
 	"github.com/celestiaorg/celestia-app/test/util/testnode"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	bank "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
+	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/libs/rand"
 )
 
@@ -68,10 +71,111 @@ func (s *SignerTestSuite) TestSubmitTx() {
 	require.EqualValues(t, 0, resp.Code)
 }
 
-func (s *SignerTestSuite) ConfirmTxTimeout() {
-	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
-	defer cancel()
-	_, err := s.signer.ConfirmTx(ctx, string("E32BD15CAF57AF15D17B0D63CF4E63A9835DD1CEBB059C335C79586BC3013728"))
-	require.Error(s.T(), err)
-	require.Equal(s.T(), err, context.DeadlineExceeded)
+func (s *SignerTestSuite) TestConfirmTx() {
+	t := s.T()
+
+	fee := user.SetFee(1e6)
+	gas := user.SetGasLimit(1e6)
+
+	t.Run("deadline exceeded when the context times out", func(t *testing.T) {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+		defer cancel()
+		_, err := s.signer.ConfirmTx(ctx, "E32BD15CAF57AF15D17B0D63CF4E63A9835DD1CEBB059C335C79586BC3013728")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), context.DeadlineExceeded.Error())
+	})
+
+	t.Run("should error when tx is not found", func(t *testing.T) {
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		_, err := s.signer.ConfirmTx(ctx, "not found tx")
+		assert.Error(t, err)
+	})
+
+	t.Run("should success when tx is found immediately", func(t *testing.T) {
+		msg := bank.NewMsgSend(s.signer.Address(), testfactory.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 10)))
+		resp, err := s.submitTxWithoutConfirm([]sdk.Msg{msg}, fee, gas)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		resp, err = s.signer.ConfirmTx(s.ctx.GoContext(), resp.TxHash)
+		assert.NoError(t, err)
+		assert.Equal(t, abci.CodeTypeOK, resp.Code)
+	})
+
+	t.Run("should error when tx is found with a non-zero error code", func(t *testing.T) {
+		balance := s.queryCurrentBalance(t)
+		// Create a msg send with out of balance, ensure this tx fails
+		msg := bank.NewMsgSend(s.signer.Address(), testfactory.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 1+balance)))
+		resp, err := s.submitTxWithoutConfirm([]sdk.Msg{msg}, fee, gas)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		resp, err = s.signer.ConfirmTx(s.ctx.GoContext(), resp.TxHash)
+		assert.Error(t, err)
+		assert.NotEqual(t, abci.CodeTypeOK, resp.Code)
+	})
+}
+
+func (s *SignerTestSuite) TestGasEstimation() {
+	msg := bank.NewMsgSend(s.signer.Address(), testfactory.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 10)))
+	gas, err := s.signer.EstimateGas(context.Background(), []sdk.Msg{msg})
+	require.NoError(s.T(), err)
+	require.Greater(s.T(), gas, uint64(0))
+}
+
+// TestGasConsumption verifies that the amount deducted from a user's balance is
+// based on the fee provided in the tx instead of the gas used by the tx. This
+// behavior leads to poor UX because tx submitters must over-estimate the amount
+// of gas that their tx will consume and they are not refunded for the excess.
+func (s *SignerTestSuite) TestGasConsumption() {
+	t := s.T()
+
+	utiaToSend := int64(1)
+	msg := bank.NewMsgSend(s.signer.Address(), testfactory.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, utiaToSend)))
+
+	gasPrice := int64(1)
+	gasLimit := uint64(1e6)
+	fee := uint64(1e6) // 1 TIA
+	// Note: gas price * gas limit = fee amount. So by setting gasLimit and fee
+	// to the same value, these options set a gas price of 1utia.
+	options := []user.TxOption{user.SetGasLimit(gasLimit), user.SetFee(fee)}
+
+	balanceBefore := s.queryCurrentBalance(t)
+	resp, err := s.signer.SubmitTx(s.ctx.GoContext(), []sdk.Msg{msg}, options...)
+	require.NoError(t, err)
+
+	require.EqualValues(t, abci.CodeTypeOK, resp.Code)
+	balanceAfter := s.queryCurrentBalance(t)
+
+	// verify that the amount deducted depends on the fee set in the tx.
+	amountDeducted := balanceBefore - balanceAfter - utiaToSend
+	assert.Equal(t, int64(fee), amountDeducted)
+
+	// verify that the amount deducted does not depend on the actual gas used.
+	gasUsedBasedDeduction := resp.GasUsed * gasPrice
+	assert.NotEqual(t, gasUsedBasedDeduction, amountDeducted)
+	// The gas used based deduction should be less than the fee because the fee is 1 TIA.
+	assert.Less(t, gasUsedBasedDeduction, int64(fee))
+}
+
+func (s *SignerTestSuite) queryCurrentBalance(t *testing.T) int64 {
+	balanceQuery := bank.NewQueryClient(s.ctx.GRPCClient)
+	balanceResp, err := balanceQuery.AllBalances(s.ctx.GoContext(), &bank.QueryAllBalancesRequest{Address: s.signer.Address().String()})
+	require.NoError(t, err)
+	return balanceResp.Balances.AmountOf(app.BondDenom).Int64()
+}
+
+func (s *SignerTestSuite) submitTxWithoutConfirm(msgs []sdk.Msg, opts ...user.TxOption) (*sdk.TxResponse, error) {
+	txBytes, err := s.signer.CreateTx(msgs, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	resp, err := s.signer.BroadcastTx(s.ctx.GoContext(), txBytes)
+	if err != nil {
+		return nil, err
+	}
+	if resp.Code != 0 {
+		return resp, fmt.Errorf("tx failed with code %d: %s", resp.Code, resp.RawLog)
+	}
+	return resp, nil
 }