Fast Lane, an open-source arbitrage protocol, allows any user to perform arbitrage between Bancor ecosystem protocols and external exchanges and redirect arbitrage profits back to the protocol.
The following methods are used for submitting transactions:
submit_transaction
submit_private_transaction
cancel_private_transaction
These methods are used inconsistently across the code, sometimes receiving a signed transaction as input, sometimes receiving an unsigned transaction as input, and sometimes receiving a raw transaction as input.
In addition to that, not every attempt to submit a transaction is complemented with an attempt to cancel it upon timeout.
This requires a higher-level decision on which transactions should be cancelled when needed, and which should not be.
Technically, submitting a transaction requires only the signed transaction as input, but cancelling it requires the original (unsigned) transaction which was used in order to create it, as input.
So designing a solution for this problem should also take into account how every submitted transaction which may need to be cancelled, can be easily cancelled.
An addition bug in the cancellation method, is that it resets the transaction's data attribute but not its value attribute.
As a result, cancelling a transaction which includes ETH, might end up with that transaction being only "partially cancelled" (namely, the ETH-transfer part in that transaction will effectively take place despite the cancellation).
The aforementioned methods are used only in one place outside of this module, in method generate_shutdown_tx.
So the implemented solution should also include this specific usage.
Naming and coding conventions:
A signed transaction can be referred to as signed_tx; no need to use signed_arb_tx in the method itself, as it is designated for handling any type of transaction and not just arb transactions; same goes for unsigned_tx and raw_tx; of course, in the calling method they should be named based on what they truly represent (e.g., approve_tx)
Methods submit_transaction is designated for "tenderly OR non-ethereum", while method submit_private_transaction is designated for all other cases, i.e., "non-tenderly AND ethereum"; These two methods should be named accordingly
Where tx_receipt is the dictionary returned from web3.eth.wait_for_transaction_receipt in BOTH cases.
This could potentially lead to unpredictable and undesirable behavior.
barakman
commented
9 months ago
The following methods are used for submitting transactions:
submit_transactionsubmit_private_transactioncancel_private_transactionThese methods are used inconsistently across the code, sometimes receiving a signed transaction as input, sometimes receiving an unsigned transaction as input, and sometimes receiving a raw transaction as input.
In addition to that, not every attempt to submit a transaction is complemented with an attempt to cancel it upon timeout. This requires a higher-level decision on which transactions should be cancelled when needed, and which should not be.
Technically, submitting a transaction requires only the signed transaction as input, but cancelling it requires the original (unsigned) transaction which was used in order to create it, as input.
So designing a solution for this problem should also take into account how every submitted transaction which may need to be cancelled, can be easily cancelled.
An addition bug in the cancellation method, is that it resets the transaction's
dataattribute but not itsvalueattribute. As a result, cancelling a transaction which includes ETH, might end up with that transaction being only "partially cancelled" (namely, the ETH-transfer part in that transaction will effectively take place despite the cancellation).The aforementioned methods are used only in one place outside of this module, in method
generate_shutdown_tx. So the implemented solution should also include this specific usage.Naming and coding conventions:
signed_tx; no need to usesigned_arb_txin the method itself, as it is designated for handling any type of transaction and not just arb transactions; same goes forunsigned_txandraw_tx; of course, in the calling method they should be named based on what they truly represent (e.g.,approve_tx)submit_transactionis designated for "tenderly OR non-ethereum", while methodsubmit_private_transactionis designated for all other cases, i.e., "non-tenderly AND ethereum"; These two methods should be named accordinglyAn additional issue:
submit_transactionreturnstx_receiptsubmit_private_transactionreturnstx_receipt["transactionHash"]Where
tx_receiptis the dictionary returned fromweb3.eth.wait_for_transaction_receiptin BOTH cases. This could potentially lead to unpredictable and undesirable behavior.