Add flag to skip slippage computations

[harisang]

This PR adds a flag to completely skip slippage calculations, if they are not needed.

[fhenneke]

I only glanced over the changes and they look reasonable.

Is the changed code part of any test? Or is there a simple way to manually test this change?

[harisang]

I only glanced over the changes and they look reasonable.

Is the changed code part of any test? Or is there a simple way to manually test this change?

Not yet. I pushed a change in order to make existing tests pass. I will add some test so that we can confirm the flag works as expected.

[fhenneke]

I suppose that we are not really interested in the fully general approach. Then this would work:

Suggested change

	reward_target_reduced_df_columns = [
	x for x in list(reward_target_df.columns) if x != "solver_name"
	]
	reward_target_reduced_df = reward_target_df[reward_target_reduced_df_columns]
	service_fee_reduced_df_columns = [
	x for x in list(service_fee_df.columns) if x != "solver_name"
	]
	service_fee_reduced_df = service_fee_df[service_fee_reduced_df_columns]
	reward_target_reduced_df = reward_target_df.drop("solver_name", axis=1)
	service_fee_reduced_df = service_fee_df.drop("solver_name", axis=1)

If we actually want that generality, this and other data transformations should probably happen somewhere else. E.g. right after fetching the data.

[harisang]

I only glanced over the changes and they look reasonable.

Is the changed code part of any test? Or is there a simple way to manually test this change?

Following up on this, I am not sure how easy it would be to add clean tests for this, so don't know how soon i will address this. This is also meant for local testing so I would say this is not urgent so let's leave this PR open until i address the testing part

[fhenneke]

Not merging this might complicate other things like deploying on arbitrum.
If you are confident that without the flag, results are not changed, we should proceed with merging.

[harisang]

Not merging this might complicate other things like deploying on arbitrum. If you are confident that without the flag, results are not changed, we should proceed with merging.

There are some issues when i did some local testing today and will try to figure out what is going on. I saw this alert
AssertionError: Slippage validation Failed with columns: {'slippage_usd', 'tx_hash', 'block_time', 'solver', 'slippage_wei'}

[fhenneke]

This error is probably due to the fact that the dummy slippage data frame does not have the required columns. Thus the check in validate_df_columns fails.

The dataframe slippage_df needs to have at least the columns SLIPPAGE_COLUMNS, i.e. {"solver", "solver_name", "eth_slippage_wei"}. The current code seems to miss solver_name and eth_slippage_wei in the case you checked.