Move KeySpecific field

[george-dorin]

• Move KeySpecificMaxGasPriceWei into GasEstimator
• Rename KeySpecificMaxGasPriceWei to KeySpecificMaxPrice to better match the config name

[github-actions]

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

[cedric-cordenier]

@george-dorin Is moving it into GasEstimator correct? According to the docs, there's this:

[[EVM.KeySpecific]]
Key = '0x2a3e23c6f242F5345320814aC8a1b4E58707D292' # Example
GasEstimator.PriceMax = '79 gwei' # Example

Which suggests that we should have cfg.EVM().KeySpecific().GasEstimator().PriceMax() or similar (KeySpecific().GasEstimatorPriceMax()?)

[jmank88]

Maybe we should consider naming this a little differently?

Suggested change

	func (g *gasEstimatorConfig) KeySpecificMaxPrice(addr gethcommon.Address) *assets.Wei {
	func (g *gasEstimatorConfig) MaxPriceKey(addr gethcommon.Address) *assets.Wei {

Suggested change

	func (g *gasEstimatorConfig) KeySpecificMaxPrice(addr gethcommon.Address) *assets.Wei {
	func (g *gasEstimatorConfig) MaxPriceByKey(addr gethcommon.Address) *assets.Wei {

Suggested change

	func (g *gasEstimatorConfig) KeySpecificMaxPrice(addr gethcommon.Address) *assets.Wei {
	func (g *gasEstimatorConfig) MaxPriceForKey(addr gethcommon.Address) *assets.Wei {

[george-dorin]

Sure, that makes more sense, renamed in b6cfd04

[jmank88]

@george-dorin Is moving it into GasEstimator correct? According to the docs, there's this:

[[EVM.KeySpecific]]
Key = '0x2a3e23c6f242F5345320814aC8a1b4E58707D292' # Example
GasEstimator.PriceMax = '79 gwei' # Example

Which suggests that we should have cfg.EVM().KeySpecific().GasEstimator().PriceMax() or similar (KeySpecific().GasEstimatorPriceMax()?)

Oh I think it get's weird because we don't just look up the field directly, we fallback to default and also constraint a bound?

[george-dorin]

@george-dorin Is moving it into GasEstimator correct? According to the docs, there's this:

[[EVM.KeySpecific]]
Key = '0x2a3e23c6f242F5345320814aC8a1b4E58707D292' # Example
GasEstimator.PriceMax = '79 gwei' # Example

Which suggests that we should have cfg.EVM().KeySpecific().GasEstimator().PriceMax() or similar (KeySpecific().GasEstimatorPriceMax()?)

Oh I think it get's weird because we don't just look up the field directly, we fallback to default and also constraint a bound?

Yeah, this field is sort of odd because of the fallback, placing it in the gasEstimator makes the most sense and leads to a much cleaner implementation.

[cedric-cordenier]

@george-dorin Makes sense! In that case, should we change the Config structure to reflect this, i.e. move KeySpecific under GasEstimator? (This is probably out of scope for this PR, but if we want to do this maybe we can put it on the backlog)

[george-dorin]

@george-dorin Makes sense! In that case, should we change the Config structure to reflect this, i.e. move KeySpecific under GasEstimator? (This is probably out of scope for this PR, but if we want to do this maybe we can put it on the backlog)

Yes, we could definitely do this, will make a ticket

[cl-sonarqube-production]

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

100.0% Coverage
2.8% Duplication