Skip to content

Commit

Permalink
[RelayMiner] Use gas for claim and proof txs (#1018)
Browse files Browse the repository at this point in the history
## Summary

This pull request includes several changes to the `pkg/client` package,
focusing on adding gas limit and gas price parameters to transaction
signing and broadcasting functions, as well as making related updates in
tests and configurations.

It also updates the `BroadcastTx` method in `pkg/client/tx/context.go`
to use `BroadcastTxSync` instead of `BroadcastTxAsync` for better error
handling during the check-tx ABCI operation.

Minor Fix: Changed the URL in `makefiles/relay.mk` to include a trailing
slash.

## Issue


![image](https://github.com/user-attachments/assets/535e99b3-621b-46bd-a335-49e167c50cba)

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [x] Bug fix
- [ ] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [x] **Unit Tests**: `make go_develop_and_test`
- [x] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [x] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [x] I have left TODOs throughout the codebase, if applicable

---------

Co-authored-by: Dima K. <[email protected]>
Co-authored-by: Dmitry K <[email protected]>
  • Loading branch information
3 people authored Jan 7, 2025
1 parent ab97af8 commit 92fbd03
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 72 deletions.
62 changes: 2 additions & 60 deletions e2e/tests/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package e2e
import (
"bytes"
"fmt"
"net"
"net/url"
"os"
"os/exec"
Expand All @@ -26,11 +25,6 @@ var (
defaultHome = os.Getenv("POKTROLLD_HOME")
// defaultPathURL used by curl commands to send relay requests
defaultPathURL = os.Getenv("PATH_URL")
// defaultPathHostOverride overrides the host in the URL used to send requests
// Since the current DevNet infrastructure does not support arbitrary subdomains,
// this is used to specify the host to connect to and the full host (with the service as a subdomain)
// will be sent in the "Host" request header.
defaultPathHostOverride = os.Getenv("PATH_HOST_OVERRIDE")
// defaultDebugOutput provides verbose output on manipulations with binaries (cli command, stdout, stderr)
defaultDebugOutput = os.Getenv("E2E_DEBUG_OUTPUT")
)
Expand Down Expand Up @@ -189,22 +183,6 @@ func (p *pocketdBin) runCurlCmd(rpcBaseURL, service, method, path, appAddr, data
return nil, err
}

// Get the virtual host that will be sent in the "Host" request header
virtualHost := getVirtualHostFromUrlForService(rpcUrl, service)

// TODO_HACK: As of PR #879, the DevNet infrastructure does not support routing
// requests to arbitrary subdomains due to TLS certificate-related complexities.
// In such environment, defaultPathHostOverride (which contains no subdomain)
// is used as:
// 1. The gateway's 'host:port' to connect to
// 2. A base to which the service is added as a subdomain then set as the "Host" request header.
// (i.e. Host: <service>.<defaultPathHostOverride>)
//
// Override the actual connection address if the environment requires it.
if defaultPathHostOverride != "" {
rpcUrl.Host = defaultPathHostOverride
}

// Ensure that if a path is provided, it starts with a "/".
// This is required for RESTful APIs that use a path to identify resources.
// For JSON-RPC APIs, the resource path should be empty, so empty paths are allowed.
Expand All @@ -225,8 +203,9 @@ func (p *pocketdBin) runCurlCmd(rpcBaseURL, service, method, path, appAddr, data
"-v", // verbose output
"-sS", // silent with error
"-H", `Content-Type: application/json`, // HTTP headers
"-H", fmt.Sprintf("Host: %s", virtualHost), // Add virtual host header
"-H", fmt.Sprintf("Host: %s", rpcUrl.Host), // Add virtual host header
"-H", fmt.Sprintf("X-App-Address: %s", appAddr),
"-H", fmt.Sprintf("target-service-id: %s", service),
rpcUrl.String(),
}

Expand Down Expand Up @@ -262,40 +241,3 @@ func (p *pocketdBin) runCurlCmd(rpcBaseURL, service, method, path, appAddr, data

return r, err
}

// formatURLString returns RESTful or JSON-RPC API endpoint URL depending
// on the parameters provided.
func formatURLString(serviceAlias, rpcUrl, path string) string {
// For JSON-RPC APIs, the path should be empty
if len(path) == 0 {
return fmt.Sprintf("http://%s.%s/v1", serviceAlias, rpcUrl)
}

// For RESTful APIs, the path should not be empty.
// We remove the leading / to make the format string below easier to read.
if path[0] == '/' {
path = path[1:]
}
return fmt.Sprintf("http://%s.%s/v1/%s", serviceAlias, rpcUrl, path)
}

// getVirtualHostFromUrlForService returns a virtual host taking into consideration
// the URL's host and the service if it's non-empty.
// Specifically, it:
// 1. Extract's the host from the rpcURL
// 2. Prefixes the service as a subdomain to (1) the given rpcUrl host stripped of the port
//
// TODO_HACK: This is needed as of PR #879 because the DevNet infrastructure does
// not support arbitrary subdomains due to TLS certificate-related complexities.
func getVirtualHostFromUrlForService(rpcUrl *url.URL, service string) string {
// Strip port if it exists and add service prefix
host, _, err := net.SplitHostPort(rpcUrl.Host)
if err != nil {
// err is non-nil if rpcUrl.Host does not have a port.
// Use the entire host as is
host = rpcUrl.Host
}
virtualHost := fmt.Sprintf("%s.%s", service, host)

return virtualHost
}
8 changes: 5 additions & 3 deletions e2e/tests/session_steps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ func (s *suite) TheUserShouldWaitForTheModuleMessageToBeSubmitted(module, msgTyp
// so that next steps that assert on supplier rewards can do it without having
// the proof submission fee skewing the results.
switch msgType {
case "CreateClaim":
fallthrough
case "SubmitProof":
supplierOperatorAddress := getMsgSubmitProofSenderAddress(event)
supplierOperatorAddress := getMsgSenderAddress(event)
require.NotEmpty(s, supplierOperatorAddress)

supplierAccName := accAddrToNameMap[supplierOperatorAddress]
Expand Down Expand Up @@ -417,8 +419,8 @@ func combineEventMatchFns(fns ...func(*abci.Event) bool) func(*abci.Event) bool
}
}

// getMsgSubmitProofSenderAddress returns the sender address from the given event.
func getMsgSubmitProofSenderAddress(event *abci.Event) string {
// getMsgSenderAddress returns the sender address from the given event.
func getMsgSenderAddress(event *abci.Event) string {
senderAttrIdx := slices.IndexFunc(event.Attributes, func(attr abci.EventAttribute) bool {
return attr.Key == "sender"
})
Expand Down
4 changes: 2 additions & 2 deletions makefiles/relay.mk
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
send_relay_path_JSONRPC: test_e2e_env ## Send a JSONRPC relay through PATH to a local anvil (test ETH) node
curl -X POST -H "Content-Type: application/json" \
-H "X-App-Address: pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4" \
-H "target-service-id: anvil" \
--data '{"jsonrpc":"2.0","method":"eth_blockNumber","params":[],"id":1}' \
http://anvil.localhost:3000/v1
# $(subst http://,http://anvil.,$(PATH_URL))/v1
http://localhost:3000/v1/

# TODO_MAINNET(@red-0ne): Re-enable this once PATH Gateway supports REST.
# See https://github.com/buildwithgrove/path/issues/87
Expand Down
7 changes: 7 additions & 0 deletions pkg/client/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ type TxContext interface {

// GetClientCtx returns the cosmos-sdk client context associated with the transaction context.
GetClientCtx() cosmosclient.Context

// GetSimulatedTxGas returns the estimated gas for the given messages.
GetSimulatedTxGas(
ctx context.Context,
signingKeyName string,
msgs ...cosmostypes.Msg,
) (uint64, error)
}

// Block is an interface which abstracts the details of a block to its minimal
Expand Down
28 changes: 26 additions & 2 deletions pkg/client/tx/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (
"sync"

"cosmossdk.io/depinject"
"cosmossdk.io/math"
abci "github.com/cometbft/cometbft/abci/types"
"github.com/cometbft/cometbft/libs/json"
rpctypes "github.com/cometbft/cometbft/rpc/jsonrpc/types"
comettypes "github.com/cometbft/cometbft/types"
cosmostypes "github.com/cosmos/cosmos-sdk/types"
"go.uber.org/multierr"

"github.com/pokt-network/poktroll/app/volatile"
"github.com/pokt-network/poktroll/pkg/client"
"github.com/pokt-network/poktroll/pkg/client/events"
"github.com/pokt-network/poktroll/pkg/client/keyring"
Expand Down Expand Up @@ -103,6 +105,9 @@ type txClient struct {
// that they have not already by the given timeout height.
txTimeoutPool txTimeoutPool

// gasPrices is the gas unit prices used for sending transactions.
gasPrices cosmostypes.DecCoins

// connRetryLimit is the number of times the underlying replay client
// should retry in the event that it encounters an error or its connection is interrupted.
// If connRetryLimit is < 0, it will retry indefinitely.
Expand Down Expand Up @@ -229,6 +234,12 @@ func (txnClient *txClient) SignAndBroadcast(
return either.SyncErr(validationErrs)
}

// Simulate the transaction to calculate the gas limit.
gasLimit, simErr := txnClient.txCtx.GetSimulatedTxGas(ctx, txnClient.signingKeyName, msgs...)
if simErr != nil {
return either.SyncErr(simErr)
}

// Construct the transactions using cosmos' transactions builder.
txBuilder := txnClient.txCtx.NewTxBuilder()
if err := txBuilder.SetMsgs(msgs...); err != nil {
Expand All @@ -240,8 +251,21 @@ func (txnClient *txClient) SignAndBroadcast(
timeoutHeight := txnClient.blockClient.LastBlock(ctx).
Height() + txnClient.commitTimeoutHeightOffset

// TODO_TECHDEBT: this should be configurable
txBuilder.SetGasLimit(690000042)
txBuilder.SetGasLimit(gasLimit)

gasLimitDec := math.LegacyNewDec(int64(gasLimit))
feeAmountDec := txnClient.gasPrices.MulDec(gasLimitDec)

feeCoins, changeCoins := feeAmountDec.TruncateDecimal()
// Ensure that any decimal remainder is added to the corresponding coin as a
// whole number.
// Since changeCoins is the result of DecCoins#TruncateDecimal, it will always
// be less than 1 unit of the feeCoins.
if !changeCoins.IsZero() {
feeCoins = feeCoins.Add(cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 1))
}
txBuilder.SetFeeAmount(feeCoins)

txBuilder.SetTimeoutHeight(uint64(timeoutHeight))

// sign transactions
Expand Down
40 changes: 39 additions & 1 deletion pkg/client/tx/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ func (txCtx cosmosTxContext) EncodeTx(txBuilder cosmosclient.TxBuilder) ([]byte,
// ABCI operation completes and returns a TxResponse of the transaction status at that point in time.
func (txCtx cosmosTxContext) BroadcastTx(txBytes []byte) (*cosmostypes.TxResponse, error) {
clientCtx := cosmosclient.Context(txCtx.clientCtx)
return clientCtx.BroadcastTxAsync(txBytes)
// BroadcastTxSync is used to capture any transaction error that occurs during
// the check-tx ABCI operation, otherwise errors would not be returned.
return clientCtx.BroadcastTxSync(txBytes)
}

// QueryTx queries the transaction based on its hash and optionally provides proof
Expand All @@ -103,3 +105,39 @@ func (txCtx cosmosTxContext) QueryTx(
func (txCtx cosmosTxContext) GetClientCtx() cosmosclient.Context {
return cosmosclient.Context(txCtx.clientCtx)
}

// GetSimulatedTxGas calculates the gas for the given messages using the simulation mode.
func (txCtx cosmosTxContext) GetSimulatedTxGas(
ctx context.Context,
signingKeyName string,
msgs ...cosmostypes.Msg,
) (uint64, error) {
clientCtx := cosmosclient.Context(txCtx.clientCtx)
keyRecord, err := txCtx.GetKeyring().Key(signingKeyName)
if err != nil {
return 0, err
}

accAddress, err := keyRecord.GetAddress()
if err != nil {
return 0, err
}

accountRetriever := txCtx.clientCtx.AccountRetriever
_, seq, err := accountRetriever.GetAccountNumberSequence(clientCtx, accAddress)
if err != nil {
return 0, err
}

txf := txCtx.txFactory.
WithSimulateAndExecute(true).
WithFromName(signingKeyName).
WithSequence(seq)

_, gas, err := cosmostx.CalculateGas(txCtx.GetClientCtx(), txf, msgs...)
if err != nil {
return 0, err
}

return gas, nil
}
13 changes: 12 additions & 1 deletion pkg/client/tx/options.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package tx

import "github.com/pokt-network/poktroll/pkg/client"
import (
cosmostypes "github.com/cosmos/cosmos-sdk/types"

"github.com/pokt-network/poktroll/pkg/client"
)

// WithCommitTimeoutBlocks sets the timeout duration in terms of number of blocks
// for the client to wait for broadcast transactions to be committed before
Expand Down Expand Up @@ -28,3 +32,10 @@ func WithConnRetryLimit(limit int) client.TxClientOption {
client.(*txClient).connRetryLimit = limit
}
}

// WithGasPrices sets the gas price to be used when constructing transactions.
func WithGasPrices(gasPrices cosmostypes.DecCoins) client.TxClientOption {
return func(client client.TxClient) {
client.(*txClient).gasPrices = gasPrices
}
}
34 changes: 31 additions & 3 deletions pkg/deps/config/suppliers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ package config

import (
"context"
"fmt"
"net/url"

"cosmossdk.io/depinject"
sdkclient "github.com/cosmos/cosmos-sdk/client"
cosmosflags "github.com/cosmos/cosmos-sdk/client/flags"
cosmostypes "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/gogoproto/grpc"
"github.com/spf13/cobra"

"github.com/pokt-network/poktroll/app/volatile"
"github.com/pokt-network/poktroll/pkg/client/block"
"github.com/pokt-network/poktroll/pkg/client/delegation"
"github.com/pokt-network/poktroll/pkg/client/events"
Expand Down Expand Up @@ -350,11 +353,21 @@ func NewSupplySupplierClientsFn(signingKeyNames []string) SupplierFn {
return func(
ctx context.Context,
deps depinject.Config,
_ *cobra.Command,
cmd *cobra.Command,
) (depinject.Config, error) {
gasPriceStr, err := cmd.Flags().GetString(cosmosflags.FlagGasPrices)
if err != nil {
return nil, err
}

gasPrices, err := cosmostypes.ParseDecCoins(gasPriceStr)
if err != nil {
return nil, err
}

suppliers := supplier.NewSupplierClientMap()
for _, signingKeyName := range signingKeyNames {
txClientDepinjectConfig, err := newSupplyTxClientsFn(ctx, deps, signingKeyName)
txClientDepinjectConfig, err := newSupplyTxClientsFn(ctx, deps, signingKeyName, gasPrices)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -466,12 +479,27 @@ func NewSupplyBankQuerierFn() SupplierFn {

// newSupplyTxClientFn returns a new depinject.Config which is supplied with
// the given deps and the new TxClient.
func newSupplyTxClientsFn(ctx context.Context, deps depinject.Config, signingKeyName string) (depinject.Config, error) {
func newSupplyTxClientsFn(
ctx context.Context,
deps depinject.Config,
signingKeyName string,
gasPrices cosmostypes.DecCoins,
) (depinject.Config, error) {
// Ensure that the gas prices include upokt
for _, gasPrice := range gasPrices {
if gasPrice.Denom != volatile.DenomuPOKT {
// TODO_TECHDEBT(red-0ne): Allow other gas prices denominations once supported (e.g. mPOKT, POKT)
// See https://docs.cosmos.network/main/build/architecture/adr-024-coin-metadata#decision
return nil, fmt.Errorf("only gas prices with %s denom are supported", volatile.DenomuPOKT)
}
}

txClient, err := tx.NewTxClient(
ctx,
deps,
tx.WithSigningKeyName(signingKeyName),
tx.WithCommitTimeoutBlocks(tx.DefaultCommitTimeoutHeightOffset),
tx.WithGasPrices(gasPrices),
)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/relayer/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ for such operations.`,
cmd.Flags().Bool(cosmosflags.FlagGRPCInsecure, true, "Used to initialize the Cosmos query context with grpc security options. It can be used to override the `QueryNodeGRPCInsecure` field in the config file if specified.")
cmd.Flags().String(cosmosflags.FlagChainID, "poktroll", "The network chain ID")
cmd.Flags().StringVar(&flagLogLevel, cosmosflags.FlagLogLevel, "debug", "The logging level (debug|info|warn|error)")
cmd.Flags().Float64(cosmosflags.FlagGasAdjustment, 1.5, "The adjustment factor to be multiplied by the gas estimate returned by the tx simulation")
cmd.Flags().String(cosmosflags.FlagGasPrices, "1upokt", "Set the gas unit price in upokt")

return cmd
}
Expand Down
4 changes: 4 additions & 0 deletions testutil/testclient/testtx/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ func NewOneTimeErrTxTimeoutTxContext(
},
).Times(1)

txCtxMock.EXPECT().GetSimulatedTxGas(gomock.Any(), gomock.Any(), gomock.Any()).
Return(uint64(1), nil).
Times(1)

txCtxMock.EXPECT().QueryTx(
gomock.AssignableToTypeOf(context.Background()),
gomock.AssignableToTypeOf([]byte{}),
Expand Down

0 comments on commit 92fbd03

Please sign in to comment.