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

Class TxHelpers Revision #529

Closed
wants to merge 25 commits into from
Closed

Class TxHelpers Revision #529

wants to merge 25 commits into from

Conversation

barakman
Copy link
Collaborator

@barakman barakman commented Apr 9, 2024

This PR includes several changes, mostly around coding simplification and performance improvements in the TxHelpers class, alongside a few related or partially-related cleanups in other modules.

The main changes in the TxHelpers class are listed below, and only two of them - 1 and 6 - are expected to impact the actual Arb-Behavior of the bot:

  1. Replace the user-configurable DEFAULT_GAS_PRICE_OFFSET constant with a gas_strategy function.
  2. Fetch the maxPriorityFee via web3 instead of by sending an HTTP request to the node.
  3. Fetch the gasPrice instead of fetching the entire block in order to retrieve the baseFee.
  4. Send a single gas-estimation request to the node.
  5. Fetch the accessList via web3 instead of by sending an HTTP request to the node.
  6. Remove the EXPECTED_GAS_MODIFIER constant used for producing an optimistic gas consumption.
  7. Remove support for legacy-transactions; all chains are assumed to be supporting EIP-1559.
  8. Send eth_sendPrivateTransaction HTTP requests via web3 instead of via the node.
  9. Ensure that the access list is applied on Ethereum transactions; it is not applied in the current version due to a bug in the code, and even after this PR, it will not be tested within Tenderly (when the target chain is Ethereum) because of that same bug; see issue NETWORK_TENDERLY overrides the original NETWORK #519.
  10. Construct the tx dictionary manually and then set its gas attribute by calling estimate_gas, instead of doing both of these things via build_transaction; following that, set the accessList attribute in this dictionary, and possibly update its gas attribute, by calling create_access_list; the gas-fee attributes (maxFeePerGas and maxPriorityFeePerGas) are set only at the end, in order to avoid getting the "max fee per gas less than block base fee" error.

@Lesigh-3100
Copy link
Collaborator

Beyond code review, this PR should be live-tested - including normal & private transactions.

@barakman
Copy link
Collaborator Author

barakman commented Apr 9, 2024

Beyond code review, this PR should be live-tested - including normal & private transactions.

Has been and is being, for the past few weeks

zavelevsky
zavelevsky previously approved these changes Apr 11, 2024
Copy link
Contributor

@zavelevsky zavelevsky left a comment

Choose a reason for hiding this comment

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

code looks good to me, I didn't test it and I'd like more eyes other than mine

@barakman
Copy link
Collaborator Author

Reposted via #542

@barakman barakman closed this Apr 11, 2024
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.

5 participants