Reduce alerts for cost coverage #99
Conversation
harisang
changed the title
There was a problem hiding this comment.
This PR is an update to the cost coverage test to the zero-signed fee world. It also updates alert thresholds.
I added minor comments. We can address them now or just merge this as is and add an issue to change it later.
I added a test to make sure the logic changes work. The test can be run locally using e.g. python -m tests.e2e.cost_coverage_test. It should print the error message and the test should pass.
| @@ -37,20 +36,12 @@ def cost_coverage(self, competition_data: dict[str, Any], gas_cost: float) -> bo | |||
| orders = solution["orders"] | |||
| native_prices = competition_data["auction"]["prices"] | |||
| total_fee = 0.0 | |||
| zero_signed_fee_market = False | |||
There was a problem hiding this comment.
This was removed since all orders are zero signed fees, right? Maybe the test can be renamed then to just cost coverage.
There was a problem hiding this comment.
Yeah good point. Let me try to clean it up
| if total_fee - gas_cost > 0.02: | ||
| self.alert( | ||
| f'"Fees - gasCost" is {total_fee - gas_cost} \ | ||
| for {competition_data["transactionHash"]}.' | ||
| ) | ||
| ) | ||
| elif total_fee - gas_cost < -0.04: | ||
| self.logger.info( | ||
| f'"Fees - gasCost" is {total_fee - gas_cost} \ | ||
| for {competition_data["transactionHash"]}.' | ||
| ) |
There was a problem hiding this comment.
This log message is quite different from what other tests emit. It also does not follow the log_output = ... and self.alert(log_output) convention.
|
Closes #101. |
This PR reduces alerts for cost coverage as they were too noisy, and only alerts if there is significant overcharging. There rest is just logged; the rationale being that with CIP-38, the protocol shouldn't in principle worry about cost coverage unless there is overcharging. This dune query can help with monitoring whenever needed.
https://dune.com/queries/3431685
Note that protocol fees can cause some false positives