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.

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

change default for -autoAdjustPrice to false #3109

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eliteprox
Copy link
Collaborator

What does this pull request do? Explain your changes. (required)

Specific updates (required)

How did you test each of these updates (required)

Does this pull request close any open issues?

Checklist:

@rickstaa
Copy link
Contributor

@eliteprox thanks for creating this pull request. What do you think about applying @Titan-Node's other suggestion of setting a maximum gas price the orchestrator accepts. This will prevent people from paying infinite gas in the worst case scenario 🤔.

@eliteprox
Copy link
Collaborator Author

@eliteprox thanks for creating this pull request. What do you think about applying @Titan-Node's other suggestion of setting a maximum gas price the orchestrator accepts. This will prevent people from paying infinite gas in the worst case scenario 🤔.

I believe that is already implemented with the -maxGasPrice flag. This works the same for both orchestrator and gateway.

cfg.MaxGasPrice = flag.Int("maxGasPrice", *cfg.MaxGasPrice, "Maximum gas price (priority fee + base fee) for ETH transactions in wei, 40 Gwei = 40000000000")

@Titan-Node
Copy link

I think the conversation around the max has price had something to do with the orchestrator rejecting work based on a gas spike regardless of the -maxGasPrice flag. I think the max gas flag is only for redeeming tickets but someone mentioned an Orch would still reject work if gas spiked. Maybe it was around the ticket EV changing or some other parameter that is affected with gas fluctuation? Is there a way we could test spiking the gas and making sure the Orch still accepts the work?
Would this be done with a unit test?

@ad-astra-video
Copy link
Collaborator

ad-astra-video commented Aug 3, 2024

I think the avgGasPrice values need to be updated to help Orchestrators not reject work during gas price spikes.

When we switched to arbitrum the the gas price was closer to 1 gwei and 3 gwei was used for average gas price to be conservative. Now gas price is 0.01 usually.

The other factor in the average transaction cost gas used is a hard coded number at 1,200,000 that was appropriate at time of switch to arbitrum. This may be able to be reduced. I also see the gas used varying wildly on some ticket redemptions driving cost well above the value of the ticket. Maybe there is more going on here that should be looked at to confirm we are still calculating correctly after the changes to arbitrum over the last couple years

$262 0x14a0c5171b9451ddd83073971f43d4f417476e956daf31ad1c9c6f58e3247a4d
$259 0x1dbf66240b878997b1ee4ce23bd033bbd767c765c1351653b7c0ea1136c2b1ed
$16 0x030927eb41dc0030b373c9b9d289a26829e929378f38e7bae1f75a7b534adf38
$10 0x1483f1b3ec7ecdbb0772cc2b696161c2f09bf879933e6b32db15e145c44e011d

redeemGasL2 = 1200000

var avgGasPrice = new(big.Int).Mul(big.NewInt(3), new(big.Int).Exp(big.NewInt(10), big.NewInt(9), nil))

if faceValue.Cmp(txCost) < 0 && faceValue.Cmp(r.txCostWithGasPrice(avgGasPrice)) < 0 {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants