submit solution only if solution is better than the solution in the chain

[josojo]

As title

---

Testplan:
unit tests

[josojo]

I was searching here for a more elegant solution, but I can not find it.

Thread discussing it:
rust-lang/rfcs#929 (comment)

[fleupold]

You are comparing surplus (should probably be renamed to utility in a separate PR) with the current_objective_value (which at the moment is volume). I don't think this works in the current setting as a solution with little surplus would not be corrected as long as it had more volume than a new solution had utility. Can't we compute and compare the proper volume instead?

As for how to make this more elegant with less nested if statements, you could have a helper method that returns a SolutionSubmission enum (e.g. Trivial, NotGoodEnough, Submit) and then have a match statement with only one level.

[josojo]

I thought, although this is not the correct solution, it is better than checking just that is is not the trivial solution.
Not sure, whether it is worth to code the volume formula ( actually the fee formula), as the rounding that would be needed to be considered is non-trivial.

But yeah, I am happy to pause this PR and keep it for later, once Tom's solver and the smart contract calculate the same objective value.

[nlordell]

You could use match statements with conditional patterns, something like this:

        let submitted = match solution.surplus {
            Some(surplus) if current_objective_value < surplus => {
                self.contract.submit_solution(batch, orders, solution)?;
                info!("Successfully applied solution to batch {}", batch);
                true
            },
            _ => {
                info!("Not submitting trivial solution for batch {}", batch);
                false
            },
        };

[nlordell]

You could also use Option::map and do something like:

        let submitted = if Some(true) = solution.surplus.map(|surplus| current_objective_value < surplus) {
            self.contract.submit_solution(batch, orders, solution)?;
            info!("Successfully applied solution to batch {}", batch);
            true
        } else {
            info!("Not submitting trivial solution for batch {}", batch);
            false
        };

[bh2smith]

Looks pretty good, made one inline comment about what appears to be duplicate code!

[fleupold]

I like the purpose of the PR. Right now we are comparing utility with volume, which we should probably fix before merging.

[fleupold]

You are comparing surplus (should probably be renamed to utility in a separate PR) with the current_objective_value (which at the moment is volume). I don't think this works in the current setting as a solution with little surplus would not be corrected as long as it had more volume than a new solution had utility. Can't we compute and compare the proper volume instead?

As for how to make this more elegant with less nested if statements, you could have a helper method that returns a SolutionSubmission enum (e.g. Trivial, NotGoodEnough, Submit) and then have a match statement with only one level.

[fleupold]

This log message seems off.

[bh2smith]

I agree. Also, this wouldn't be possible anyway.

[fleupold]

How about an objective value of 0? Would that ever get submitted?

[bh2smith]

At the moment, the contract does not allow for trivial solution submission. Furthermore, it does not accept solutions with zero objective evaluation either.

[josojo]

This PR is no longer necessary, as #296 will just prevent submitting solutions with less objective value