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.

Sign up for GitHub

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

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

Closed
wants to merge 4 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f827d9
working, but non-elegant solution
josojo Oct 4, 2019
ff955ab
missing print statement
josojo Oct 4, 2019
85a9001
Merge branch 'master' into onlySubmitSolutionIfBetterSolution
josojo Oct 25, 2019
2677c80
small renaming corrections and adaption of tests
josojo Oct 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions driver/src/contracts/stablex_contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ impl StableXContractImpl {

pub trait StableXContract {
fn get_current_auction_index(&self) -> Result<U256>;
fn get_current_objective_value(&self) -> Result<U256>;
fn get_auction_data(&self, _index: U256) -> Result<(AccountState, Vec<Order>)>;
fn submit_solution(
&self,
Expand All @@ -52,6 +53,20 @@ impl StableXContract for StableXContractImpl {
.map_err(DriverError::from)
}

fn get_current_objective_value(&self) -> Result<U256> {
self.base
.contract
.query(
"getCurrentObjectiveValue",
(),
None,
Options::default(),
None,
)
.wait()
.map_err(DriverError::from)
}

fn get_auction_data(&self, index: U256) -> Result<(AccountState, Vec<Order>)> {
let packed_auction_bytes: Vec<u8> = self
.base
Expand Down Expand Up @@ -189,6 +204,7 @@ pub mod tests {
#[derive(Clone)]
pub struct StableXContractMock {
pub get_current_auction_index: Mock<(), Result<U256>>,
pub get_current_objective_value: Mock<(), Result<U256>>,
pub get_auction_data: Mock<U256, Result<(AccountState, Vec<Order>)>>,
pub submit_solution: Mock<SubmitSolutionArguments, Result<()>>,
}
Expand All @@ -200,6 +216,10 @@ pub mod tests {
"Unexpected call to get_current_auction_index",
ErrorKind::Unknown,
))),
get_current_objective_value: Mock::new(Err(DriverError::new(
"Unexpected call to get_current_objective_value",
ErrorKind::Unknown,
))),
get_auction_data: Mock::new(Err(DriverError::new(
"Unexpected call to get_auction_data",
ErrorKind::Unknown,
Expand All @@ -216,6 +236,9 @@ pub mod tests {
fn get_current_auction_index(&self) -> Result<U256> {
self.get_current_auction_index.called(())
}
fn get_current_objective_value(&self) -> Result<U256> {
self.get_current_objective_value.called(())
}
fn get_auction_data(&self, index: U256) -> Result<(AccountState, Vec<Order>)> {
self.get_auction_data.called(index)
}
Expand Down
79 changes: 72 additions & 7 deletions driver/src/driver/stablex_driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,21 @@ impl<'a> StableXDriver<'a> {
solution
};

let submitted = if solution.executed_sell_amounts.iter().sum::<u128>() > 0 {
self.contract.submit_solution(batch, orders, solution)?;
info!("Successfully applied solution to batch {}", batch);
true
let current_objective_value = self.contract.get_current_objective_value()?;
let submitted = if let Some(objective_value) = solution.objective_value {
if current_objective_value < objective_value {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

self.contract.submit_solution(batch, orders, solution)?;
info!("Successfully applied solution to batch {}", batch);
true
} else {
info!("Not submitting trivial solution for batch {}", batch);
Copy link
Contributor

Choose a reason for hiding this comment

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

This log message seems off.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

false
}
} else {
info!("Not submitting trivial solution for batch {}", batch);
false
};

self.past_auctions.insert(batch);
Ok(submitted)
}
Expand Down Expand Up @@ -83,14 +90,18 @@ mod tests {
.get_auction_data
.given(batch - 1)
.will_return(Ok((state.clone(), orders.clone())));
contract
.get_current_objective_value
.given(())
.will_return(Ok(U256::zero()));

contract
.submit_solution
.given((batch - 1, Val(orders.clone()), Any))
.will_return(Ok(()));

let solution = Solution {
objective_value: Some(U256::zero()),
objective_value: Some(U256::one()),
prices: vec![1, 2],
executed_sell_amounts: vec![0, 2],
executed_buy_amounts: vec![0, 2],
Expand Down Expand Up @@ -122,13 +133,18 @@ mod tests {
.given(batch - 1)
.will_return(Ok((state.clone(), orders.clone())));

contract
.get_current_objective_value
.given(())
.will_return(Ok(U256::zero()));

contract
.submit_solution
.given((batch - 1, Val(orders.clone()), Any))
.will_return(Ok(()));

let solution = Solution {
objective_value: Some(U256::zero()),
objective_value: Some(U256::one()),
prices: vec![1, 2],
executed_sell_amounts: vec![0, 2],
executed_buy_amounts: vec![0, 2],
Expand All @@ -145,6 +161,45 @@ mod tests {
//Second auction
assert_eq!(driver.run().unwrap(), false);
}
#[test]

fn does_not_process_solution_if_better_one_exists() {
let contract = StableXContractMock::default();
let mut pf = PriceFindingMock::default();

let orders = vec![create_order_for_test(), create_order_for_test()];
let state = create_account_state_with_balance_for(&orders);

let batch = U256::from(42);
contract
.get_current_auction_index
.given(())
.will_return(Ok(batch));

contract
.get_auction_data
.given(batch - 1)
.will_return(Ok((state.clone(), orders.clone())));

contract
.get_current_objective_value
.given(())
.will_return(Ok(U256::from(100)));

let solution = Solution {
objective_value: Some(U256::one()),
prices: vec![1, 2],
executed_sell_amounts: vec![0, 2],
executed_buy_amounts: vec![0, 2],
};
pf.find_prices
.given((orders, state))
.will_return(Ok(solution));

let mut driver = StableXDriver::new(&contract, &mut pf);

assert_eq!(driver.run().unwrap(), false);
}

#[test]
fn test_errors_on_failing_contract() {
Expand Down Expand Up @@ -199,14 +254,19 @@ mod tests {
.given(())
.will_return(Ok(batch));

contract
.get_current_objective_value
.given(())
.will_return(Ok(U256::zero()));

contract
.get_auction_data
.given(batch - 1)
.will_return(Ok((state.clone(), orders.clone())));

let mut driver = StableXDriver::new(&contract, &mut pf);

assert!(driver.run().is_ok());
assert_eq!(driver.run().unwrap(), false);
assert!(!mock_it::verify(
pf.find_prices.was_called_with((orders, state))
));
Expand All @@ -226,6 +286,11 @@ mod tests {
.given(())
.will_return(Ok(batch));

contract
.get_current_objective_value
.given(())
.will_return(Ok(U256::zero()));

contract
.get_auction_data
.given(batch - 1)
Expand Down