feat(wallet)!: allowing client to set custom values for fees, nonce, gas #6124
+718
−258
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like you have BREAKING CHANGES in your PR. Check-list
|
Jenkins BuildsClick to see older builds (24)
|
saledjenic
changed the title
alaibe
approved these changes
Nov 26, 2024
| @@ -55,6 +55,9 @@ type RouteInputParams struct { | |||
| PublicKey string `json:"publicKey"` | |||
| PackID *hexutil.Big `json:"packID"` | |||
|
|
|||
| // Used internally | |||
| PathTxCustomParams map[string]*PathTxCustomParams `json:"-"` | |||
There was a problem hiding this comment.
if used internally it could be lower case?
that way it won't even be exported in json
There was a problem hiding this comment.
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.
| RouterInputParamsUuid string `json:"routerInputParamsUuid" validate:"required"` | ||
| PathName string `json:"pathName" validate:"required"` | ||
| ChainID uint64 `json:"chainID" validate:"required"` | ||
| ApprovalTx bool `json:"approvalTx"` |
| @@ -50,23 +56,28 @@ type SuggestedFeesGwei struct { | |||
| MaxFeePerGasLow *big.Float `json:"maxFeePerGasLow"` | |||
| MaxFeePerGasMedium *big.Float `json:"maxFeePerGasMedium"` | |||
| MaxFeePerGasHigh *big.Float `json:"maxFeePerGasHigh"` | |||
There was a problem hiding this comment.
shall we adjust our various fees level?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I would suggest: high to be 2x base fee and medium to be 1.2 base fee
| return r.setCustomTxDetails(pathTxIdentity, &requests.PathTxCustomParams{GasFeeMode: feeMode}) | ||
| } | ||
|
|
||
| func (r *Router) SetCustomTxDetails(ctx context.Context, pathTxIdentity *requests.PathTxIdentity, pathTxCustomParams *requests.PathTxCustomParams) error { |
There was a problem hiding this comment.
Could we find a better name for the private function? IMO a bit unclear
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
saledjenic
force-pushed
the
feat/custom-fees
branch
2 times, most recently
from
38e658a
to
f15bd8c
Compare
saledjenic
requested review from
shivekkhurana,
dlipicar,
antdanchenko,
briansztamfater and
smohamedjavid
saledjenic
force-pushed
the
feat/custom-fees
branch
from
f15bd8c
to
011767c
Compare
Removed properties from `Path` type: - `MaxFeesPerGas`, based on the sending flow progress appropriate new properties (`TxMaxFeesPerGas`, `ApprovalMaxFeesPerGas`) should be used Added new properties to `Path` type: - `RouterInputParamsUuid`, used to identify from which router input params this path was created - `TxNonce`, used to set nonce for the tx - `TxMaxFeesPerGas`, used to set max fees per gas for the tx - `TxEstimatedTime`, used to estimate time for executing the tx - `ApprovalTxNonce`, used to set nonce for the approval tx - `ApprovalTxMaxFeesPerGas`, used to set max fees per gas for the approval tx - `ApprovalTxEstimatedTime`, used to estimate time for executing the approval tx New request types added: - `PathTxCustomParams`, used to pass tx custom params from the client - `PathTxIdentity`, used to uniquely identify path (tx) to which the custom params need to be applied New endpoints added: - `SetFeeMode` used to set fee mode (`GasFeeLow`, `GasFeeMedium` or `GasFeeHigh`) - `SetCustomTxDetails` used to set custom fee mode (`SetCustomTxDetails`), if this mode is set, client needs to provide: - Max fees per gas - Max priority fee - Nonce - Gas amount
saledjenic
force-pushed
the
feat/custom-fees
branch
from
011767c
to
aae6e8b
Compare
1 task
briansztamfater
approved these changes
Dec 2, 2024
smohamedjavid
approved these changes
Dec 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removed properties from
Pathtype:MaxFeesPerGas, based on the sending flow progress appropriate new properties (TxMaxFeesPerGas,ApprovalMaxFeesPerGas) should be usedAdded new properties to
Pathtype:RouterInputParamsUuid, used to identify from which router input params this path was createdTxNonce, used to set nonce for the txTxMaxFeesPerGas, used to set max fees per gas for the txTxEstimatedTime, used to estimate time for executing the txApprovalTxNonce, used to set nonce for the approval txApprovalTxMaxFeesPerGas, used to set max fees per gas for the approval txApprovalTxEstimatedTime, used to estimate time for executing the approval txNew request types added:
PathTxCustomParams, used to pass tx custom params from the clientPathTxIdentity, used to uniquely identify the path (tx) to which the custom params need to be appliedNew endpoints added:
SetFeeMode, used to set fee mode (GasFeeLow,GasFeeMediumorGasFeeHigh)SetCustomTxDetails, used to set custom fee mode (SetCustomTxDetails), if this mode is set, the client needs to provide:Closes #6109
Closes backed part for