KS-398 gas limit on tx meta #14214
Merged
KS-398 gas limit on tx meta #14214
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
ettec
force-pushed
the
ks-398-gas-limit-on-tx-meta
branch
4 times, most recently
from
bf47569 to
c5e3145
Compare
bolekk
reviewed
Aug 23, 2024
| @@ -15,7 +15,7 @@ import ( | |||
| relayevmtypes "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types" | |||
| ) | |||
|
|
|||
| func NewWriteTarget(ctx context.Context, relayer *Relayer, chain legacyevm.Chain, lggr logger.Logger) (*targets.WriteTarget, error) { | |||
| func NewWriteTarget(ctx context.Context, relayer *Relayer, chain legacyevm.Chain, defaultGasLimit uint64, lggr logger.Logger) (*targets.WriteTarget, error) { | |||
There was a problem hiding this comment.
Can this limit still be overwritten per-workflow?
There was a problem hiding this comment.
yes it can, by specifying it in the config object for the write target in the workflow spec
ettec
force-pushed
the
ks-398-gas-limit-on-tx-meta
branch
2 times, most recently
from
47583fb to
c681ac6
Compare
cedric-cordenier
previously approved these changes
Aug 23, 2024
bolekk
previously approved these changes
Aug 23, 2024
bolekk
previously approved these changes
Aug 23, 2024
… default limit configurable
bolekk
force-pushed
the
ks-398-gas-limit-on-tx-meta
branch
from
3e55f72 to
e61bd9f
Compare
bolekk
approved these changes
Aug 23, 2024
|
jmank88
reviewed
Aug 23, 2024
| @@ -469,3 +469,5 @@ GasLimit = 5400000 # Default | |||
| FromAddress = '0x2a3e23c6f242F5345320814aC8a1b4E58707D292' # Example | |||
| # ForwarderAddress is the keystone forwarder contract address on chain. | |||
| ForwarderAddress = '0x2a3e23c6f242F5345320814aC8a1b4E58707D292' # Example | |||
| # DefaultGasLimit is the default gas limit for workflow transactions. | |||
| DefaultGasLimit = 400_000 # Default | |||
There was a problem hiding this comment.
Keeping modifiers as suffixes makes things a little easier to read and keeps things grouped together naturally when sorted:
Suggested change
| DefaultGasLimit = 400_000 # Default | |
| GasLimitDefault = 400_000 # Default |
There was a problem hiding this comment.
jmank88
reviewed
Aug 23, 2024
There was a problem hiding this comment.
Let's mention the new config field here
krehermann
approved these changes
Aug 23, 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.
https://smartcontract-it.atlassian.net/browse/KS-403
Change to allow the gas limit to be specified when submitting transactions to the evm chainwriter via the write target.
Also, secondary change to make the default gas limit configurable.