Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(wallet)!: allowing client to set custom values for fees, nonce, gas #6124

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
+718 −258
Open

feat(wallet)!: allowing client to set custom values for fees, nonce, gas #6124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc0d444
feat(wallet)!: allowing client to set custom values for fees, nonce, gas
saledjenic Nov 26, 2024
aae6e8b
test(wallet)_: added tests for selecting custom fees and setting cust…
saledjenic Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion services/connector/commands/send_transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ func (c *SendTransactionCommand) Execute(ctx context.Context, request RPCRequest
if !fetchedFees.EIP1559Enabled {
params.GasPrice = (*hexutil.Big)(fetchedFees.GasPrice)
} else {
params.MaxFeePerGas = (*hexutil.Big)(fetchedFees.FeeFor(fees.GasFeeMedium))
maxFees, err := fetchedFees.FeeFor(fees.GasFeeMedium)
if err != nil {
return "", err
}
params.MaxFeePerGas = (*hexutil.Big)(maxFees)
params.MaxPriorityFeePerGas = (*hexutil.Big)(fetchedFees.MaxPriorityFeePerGas)
}
}
Expand Down
15 changes: 15 additions & 0 deletions services/wallet/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,21 @@ func (api *API) StopSuggestedRoutesCalculation(ctx context.Context) {
api.s.router.StopSuggestedRoutesCalculation()
}

// SetFeeMode sets the fee mode for the provided path it should be used for setting predefined fee modes `GasFeeLow`, `GasFeeMedium` and `GasFeeHigh`
// in case of setting custom fee use `SetCustomTxDetails` function
func (api *API) SetFeeMode(ctx context.Context, pathTxIdentity *requests.PathTxIdentity, feeMode fees.GasFeeMode) error {
logutils.ZapLogger().Debug("call to SetFeeMode")

return api.s.router.SetFeeMode(ctx, pathTxIdentity, feeMode)
}

// SetCustomTxDetails sets custom tx details for the provided path, in case of setting predefined fee modes use `SetFeeMode` function
func (api *API) SetCustomTxDetails(ctx context.Context, pathTxIdentity *requests.PathTxIdentity, pathTxCustomParams *requests.PathTxCustomParams) error {
logutils.ZapLogger().Debug("call to SetCustomTxDetails")

return api.s.router.SetCustomTxDetails(ctx, pathTxIdentity, pathTxCustomParams)
}

// Generates addresses for the provided paths, response doesn't include `HasActivity` value (if you need it check `GetAddressDetails` function)
func (api *API) GetDerivedAddresses(ctx context.Context, password string, derivedFrom string, paths []string) ([]*DerivedAddress, error) {
info, err := api.s.gethManager.AccountsGenerator().LoadAccount(derivedFrom, password)
Expand Down
3 changes: 3 additions & 0 deletions services/wallet/requests/router_input_params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ type RouteInputParams struct {
PublicKey string `json:"publicKey"`
PackID *hexutil.Big `json:"packID"`

// Used internally
PathTxCustomParams map[string]*PathTxCustomParams `json:"-"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if used internally it could be lower case?
that way it won't even be exported in json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It cannot cause it's a different package. So either this or to have a function that will set this property, but more or less the same thing.


// TODO: Remove two fields below once we implement a better solution for tests
// Currently used for tests only
TestsMode bool
Expand Down
50 changes: 50 additions & 0 deletions services/wallet/requests/tx_custom_params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package requests

import (
"fmt"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/status-im/status-go/errors"
"github.com/status-im/status-go/services/wallet/router/fees"
)

var (
ErrMaxFeesPerGasRequired = &errors.ErrorResponse{Code: errors.ErrorCode("WRC-001"), Details: "maxFeesPerGas is required"}
ErrPriorityFeeRequired = &errors.ErrorResponse{Code: errors.ErrorCode("WRC-002"), Details: "priorityFee is required"}
)

type PathTxCustomParams struct {
GasFeeMode fees.GasFeeMode `json:"gasFeeMode" validate:"required"`
Nonce uint64 `json:"nonce"`
GasAmount uint64 `json:"gasAmount"`
MaxFeesPerGas *hexutil.Big `json:"maxFeesPerGas"`
PriorityFee *hexutil.Big `json:"priorityFee"`
}

type PathTxIdentity struct {
RouterInputParamsUuid string `json:"routerInputParamsUuid" validate:"required"`
PathName string `json:"pathName" validate:"required"`
ChainID uint64 `json:"chainID" validate:"required"`
IsApprovalTx bool `json:"isApprovalTx"`
}

func (p *PathTxIdentity) PathIdentity() string {
return fmt.Sprintf("%s-%s-%d", p.RouterInputParamsUuid, p.PathName, p.ChainID)
}

func (p *PathTxIdentity) TxIdentityKey() string {
return fmt.Sprintf("%s-%v", p.PathIdentity(), p.IsApprovalTx)
}

func (p *PathTxCustomParams) Validate() error {
if p.GasFeeMode != fees.GasFeeCustom {
return nil
}
if p.MaxFeesPerGas == nil {
return ErrMaxFeesPerGasRequired
}
if p.PriorityFee == nil {
return ErrPriorityFeeRequired
}
return nil
}
22 changes: 14 additions & 8 deletions services/wallet/router/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,18 @@ import (

// Abbreviation `WR` for the error code stands for Wallet Router
var (
ErrNotEnoughTokenBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-001"), Details: "not enough token balance, token: %s, chainId: %d"}
ErrNotEnoughNativeBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-002"), Details: "not enough native balance, token: %s, chainId: %d"}
ErrNativeTokenNotFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-003"), Details: "native token not found"}
ErrTokenNotFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-004"), Details: "token not found"}
ErrNoBestRouteFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-005"), Details: "no best route found"}
ErrCannotCheckBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-006"), Details: "cannot check balance"}
ErrLowAmountInForHopBridge = &errors.ErrorResponse{Code: errors.ErrorCode("WR-007"), Details: "bonder fee greater than estimated received, a higher amount is needed to cover fees"}
ErrNoPositiveBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-008"), Details: "no positive balance"}
ErrNotEnoughTokenBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-001"), Details: "not enough token balance, token: %s, chainId: %d"}
ErrNotEnoughNativeBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-002"), Details: "not enough native balance, token: %s, chainId: %d"}
ErrNativeTokenNotFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-003"), Details: "native token not found"}
ErrTokenNotFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-004"), Details: "token not found"}
ErrNoBestRouteFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-005"), Details: "no best route found"}
ErrCannotCheckBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-006"), Details: "cannot check balance"}
ErrLowAmountInForHopBridge = &errors.ErrorResponse{Code: errors.ErrorCode("WR-007"), Details: "bonder fee greater than estimated received, a higher amount is needed to cover fees"}
ErrNoPositiveBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-008"), Details: "no positive balance"}
ErrCustomFeeModeCannotBeSetThisWay = &errors.ErrorResponse{Code: errors.ErrorCode("WR-009"), Details: "custom fee mode cannot be set this way"}
ErrOnlyCustomFeeModeCanBeSetThisWay = &errors.ErrorResponse{Code: errors.ErrorCode("WR-010"), Details: "only custom fee mode can be set this way"}
ErrTxIdentityNotProvided = &errors.ErrorResponse{Code: errors.ErrorCode("WR-011"), Details: "transaction identity not provided"}
ErrTxCustomParamsNotProvided = &errors.ErrorResponse{Code: errors.ErrorCode("WR-012"), Details: "transaction custom params not provided"}
ErrCannotCustomizeIfNoRoute = &errors.ErrorResponse{Code: errors.ErrorCode("WR-013"), Details: "cannot customize params if no route"}
ErrCannotFindPathForProvidedIdentity = &errors.ErrorResponse{Code: errors.ErrorCode("WR-014"), Details: "cannot find path for provided identity"}
)
21 changes: 16 additions & 5 deletions services/wallet/router/fees/fees.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/ethereum/go-ethereum/consensus/misc"
"github.com/ethereum/go-ethereum/params"
gaspriceoracle "github.com/status-im/status-go/contracts/gas-price-oracle"
"github.com/status-im/status-go/errors"
"github.com/status-im/status-go/rpc"
"github.com/status-im/status-go/rpc/chain"
"github.com/status-im/status-go/services/wallet/common"
Expand All @@ -23,6 +24,11 @@ const (
GasFeeLow GasFeeMode = iota
GasFeeMedium
GasFeeHigh
GasFeeCustom
)

var (
ErrCustomFeeModeNotAvailableInSuggestedFees = &errors.ErrorResponse{Code: errors.ErrorCode("WRF-001"), Details: "custom fee mode is not available in suggested fees"}
)

type MaxFeesLevels struct {
Expand Down Expand Up @@ -50,23 +56,28 @@ type SuggestedFeesGwei struct {
MaxFeePerGasLow *big.Float `json:"maxFeePerGasLow"`
MaxFeePerGasMedium *big.Float `json:"maxFeePerGasMedium"`
MaxFeePerGasHigh *big.Float `json:"maxFeePerGasHigh"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we adjust our various fees level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you mean this? To have a new formula for each?

This is what we have now:

{
	Low:    (*hexutil.Big)(new(big.Int).Add(baseFee, maxPriorityFeePerGas)),
	Medium: (*hexutil.Big)(new(big.Int).Add(new(big.Int).Mul(baseFee, big.NewInt(2)), maxPriorityFeePerGas)),
	High:   (*hexutil.Big)(new(big.Int).Add(new(big.Int).Mul(baseFee, big.NewInt(3)), maxPriorityFeePerGas)),
}

Let me know if you want to change the equation and to what?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest: high to be 2x base fee and medium to be 1.2 base fee

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alaibe I've created a new PR for that, please check it here #6156

MaxFeePerGasCustom *big.Float `json:"maxFeePerGasCustom"`
L1GasFee *big.Float `json:"l1GasFee,omitempty"`
EIP1559Enabled bool `json:"eip1559Enabled"`
}

func (m *MaxFeesLevels) FeeFor(mode GasFeeMode) *big.Int {
func (m *MaxFeesLevels) FeeFor(mode GasFeeMode) (*big.Int, error) {
if mode == GasFeeCustom {
return nil, ErrCustomFeeModeNotAvailableInSuggestedFees
}

if mode == GasFeeLow {
return m.Low.ToInt()
return m.Low.ToInt(), nil
}

if mode == GasFeeHigh {
return m.High.ToInt()
return m.High.ToInt(), nil
}

return m.Medium.ToInt()
return m.Medium.ToInt(), nil
}

func (s *SuggestedFees) FeeFor(mode GasFeeMode) *big.Int {
func (s *SuggestedFees) FeeFor(mode GasFeeMode) (*big.Int, error) {
return s.MaxFeesLevels.FeeFor(mode)
}

Expand Down
4 changes: 2 additions & 2 deletions services/wallet/router/pathprocessor/processor_bridge_hop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (h *HopBridgeProcessor) sendOrBuild(sendArgs *MultipathProcessorTxArgs, sig

var nonce uint64
if lastUsedNonce < 0 {
nonce, err = h.transactor.NextNonce(h.contractMaker.RPCClient, fromChain.ChainID, sendArgs.HopTx.From)
nonce, err = h.transactor.NextNonce(context.Background(), h.contractMaker.RPCClient, fromChain.ChainID, sendArgs.HopTx.From)
if err != nil {
return tx, createBridgeHopErrorResponse(err)
}
Expand Down Expand Up @@ -363,7 +363,7 @@ func (h *HopBridgeProcessor) sendOrBuildV2(sendArgs *wallettypes.SendTxArgs, sig

var nonce uint64
if lastUsedNonce < 0 {
nonce, err = h.transactor.NextNonce(h.contractMaker.RPCClient, fromChain.ChainID, sendArgs.From)
nonce, err = h.transactor.NextNonce(context.Background(), h.contractMaker.RPCClient, fromChain.ChainID, sendArgs.From)
if err != nil {
return tx, createBridgeHopErrorResponse(err)
}
Expand Down
2 changes: 1 addition & 1 deletion services/wallet/router/pathprocessor/processor_erc1155.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (s *ERC1155Processor) sendOrBuild(sendArgs *MultipathProcessorTxArgs, signe

var nonce uint64
if lastUsedNonce < 0 {
nonce, err = s.transactor.NextNonce(s.rpcClient, sendArgs.ChainID, sendArgs.ERC1155TransferTx.From)
nonce, err = s.transactor.NextNonce(context.Background(), s.rpcClient, sendArgs.ChainID, sendArgs.ERC1155TransferTx.From)
if err != nil {
return tx, createERC1155ErrorResponse(err)
}
Expand Down
2 changes: 1 addition & 1 deletion services/wallet/router/pathprocessor/processor_erc721.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (s *ERC721Processor) sendOrBuild(sendArgs *MultipathProcessorTxArgs, signer

var nonce uint64
if lastUsedNonce < 0 {
nonce, err = s.transactor.NextNonce(s.rpcClient, sendArgs.ChainID, sendArgs.ERC721TransferTx.From)
nonce, err = s.transactor.NextNonce(context.Background(), s.rpcClient, sendArgs.ChainID, sendArgs.ERC721TransferTx.From)
if err != nil {
return tx, createERC721ErrorResponse(err)
}
Expand Down
97 changes: 74 additions & 23 deletions services/wallet/router/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type SuggestedRoutes struct {

type Router struct {
rpcClient *rpc.Client
transactor *transactions.Transactor
tokenManager *token.Manager
marketManager *market.Manager
collectiblesService *collectibles.Service
Expand Down Expand Up @@ -92,6 +93,7 @@ func NewRouter(rpcClient *rpc.Client, transactor *transactions.Transactor, token

return &Router{
rpcClient: rpcClient,
transactor: transactor,
tokenManager: tokenManager,
marketManager: marketManager,
collectiblesService: collectibles,
Expand Down Expand Up @@ -140,6 +142,57 @@ func (r *Router) SetTestBalanceMap(balanceMap map[string]*big.Int) {
}
}

func (r *Router) setCustomTxDetails(pathTxIdentity *requests.PathTxIdentity, pathTxCustomParams *requests.PathTxCustomParams) error {
if pathTxIdentity == nil {
return ErrTxIdentityNotProvided
}
if pathTxCustomParams == nil {
return ErrTxCustomParamsNotProvided
}
err := pathTxCustomParams.Validate()
if err != nil {
return err
}

r.activeRoutesMutex.Lock()
defer r.activeRoutesMutex.Unlock()
if r.activeRoutes == nil || len(r.activeRoutes.Best) == 0 {
return ErrCannotCustomizeIfNoRoute
}

for _, path := range r.activeRoutes.Best {
if path.PathIdentity() != pathTxIdentity.PathIdentity() {
continue
}

r.lastInputParamsMutex.Lock()
if r.lastInputParams.PathTxCustomParams == nil {
r.lastInputParams.PathTxCustomParams = make(map[string]*requests.PathTxCustomParams)
}
r.lastInputParams.PathTxCustomParams[pathTxIdentity.TxIdentityKey()] = pathTxCustomParams
r.lastInputParamsMutex.Unlock()

return nil
}

return ErrCannotFindPathForProvidedIdentity
}

func (r *Router) SetFeeMode(ctx context.Context, pathTxIdentity *requests.PathTxIdentity, feeMode fees.GasFeeMode) error {
if feeMode == fees.GasFeeCustom {
return ErrCustomFeeModeCannotBeSetThisWay
}

return r.setCustomTxDetails(pathTxIdentity, &requests.PathTxCustomParams{GasFeeMode: feeMode})
}

func (r *Router) SetCustomTxDetails(ctx context.Context, pathTxIdentity *requests.PathTxIdentity, pathTxCustomParams *requests.PathTxCustomParams) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we find a better name for the private function? IMO a bit unclear

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am open to suggestions, but it's not private, it's exposed via API.
There are 2 new functions that we're exposing, mentioned in the description of this PR, under "New endpoints added:" section.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this one is not private but there is also setCustomTxDetails
so 2 functions with same name one private, on public
I didn't suggest cause nothing comes to mind but ... there is probably a better naming

if pathTxCustomParams != nil && pathTxCustomParams.GasFeeMode != fees.GasFeeCustom {
return ErrOnlyCustomFeeModeCanBeSetThisWay
}
return r.setCustomTxDetails(pathTxIdentity, pathTxCustomParams)
}

func newSuggestedRoutes(
input *requests.RouteInputParams,
candidates routes.Route,
Expand Down Expand Up @@ -585,7 +638,11 @@ func (r *Router) resolveCandidates(ctx context.Context, input *requests.RouteInp
var (
testsMode = input.TestsMode && input.TestParams != nil
group = async.NewAtomicGroup(ctx)
mu sync.Mutex

candidatesMu sync.Mutex

usedNonces = make(map[uint64]uint64)
usedNoncesMu sync.Mutex
)

crossChainAmountOptions, err := r.findOptionsForSendingAmount(input, selectedFromChains)
Expand All @@ -601,17 +658,17 @@ func (r *Router) resolveCandidates(ctx context.Context, input *requests.RouteInp
zap.Uint64("toChainId", toChainID),
zap.Stringer("amount", amount),
zap.Error(err))
mu.Lock()
defer mu.Unlock()
candidatesMu.Lock()
defer candidatesMu.Unlock()
processorErrors = append(processorErrors, &ProcessorError{
ProcessorName: processorName,
Error: err,
})
}

appendPathFn := func(path *routes.Path) {
mu.Lock()
defer mu.Unlock()
candidatesMu.Lock()
defer candidatesMu.Unlock()
candidates = append(candidates, path)
}

Expand Down Expand Up @@ -765,22 +822,16 @@ func (r *Router) resolveCandidates(ctx context.Context, input *requests.RouteInp
continue
}

maxFeesPerGas := fetchedFees.FeeFor(input.GasFeeMode)

estimatedTime := r.feesManager.TransactionEstimatedTime(ctx, network.ChainID, maxFeesPerGas)
if approvalRequired && estimatedTime < fees.MoreThanFiveMinutes {
estimatedTime += 1
}

path := &routes.Path{
ProcessorName: pProcessor.Name(),
FromChain: network,
ToChain: dest,
FromToken: token,
ToToken: toToken,
AmountIn: (*hexutil.Big)(amountOption.amount),
AmountInLocked: amountOption.locked,
AmountOut: (*hexutil.Big)(amountOut),
RouterInputParamsUuid: input.Uuid,
ProcessorName: pProcessor.Name(),
FromChain: network,
ToChain: dest,
FromToken: token,
ToToken: toToken,
AmountIn: (*hexutil.Big)(amountOption.amount),
AmountInLocked: amountOption.locked,
AmountOut: (*hexutil.Big)(amountOut),

// set params that we don't want to be recalculated with every new block creation
TxGasAmount: gasLimit,
Expand All @@ -792,12 +843,12 @@ func (r *Router) resolveCandidates(ctx context.Context, input *requests.RouteInp
ApprovalContractAddress: &approvalContractAddress,
ApprovalGasAmount: approvalGasLimit,

EstimatedTime: estimatedTime,

SubtractFees: amountOption.subtractFees,
}

err = r.cacluateFees(ctx, path, fetchedFees, processorInputParams.TestsMode, processorInputParams.TestApprovalL1Fee)
usedNoncesMu.Lock()
err = r.evaluateAndUpdatePathDetails(ctx, path, fetchedFees, usedNonces, processorInputParams.TestsMode, processorInputParams.TestApprovalL1Fee)
usedNoncesMu.Unlock()
if err != nil {
appendProcessorErrorFn(pProcessor.Name(), input.SendType, processorInputParams.FromChain.ChainID, processorInputParams.ToChain.ChainID, processorInputParams.AmountIn, err)
continue
Expand Down
Loading