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.

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

Refactor: results not stored on contract #217

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ k256 = { version = "0.13", default-features = false, features = ["ecdsa"] }
libfuzzer-sys = "0.4"
rand = "0.8"
schemars = { version = "0.8", features = ["semver"] }
seda-common = { git = "https://github.com/sedaprotocol/seda-common-rs.git", branch = "refactor/types-and-renames" }
seda-common = { git = "https://github.com/sedaprotocol/seda-common-rs.git", branch = "hy/sudo-refactor" }
# leaving this in to make local development easier
# seda-common = { path = "../seda-common-rs" }
semver = { version = "1.0", features = ["serde"] }
Expand Down
2 changes: 1 addition & 1 deletion contract/src/msgs/data_requests/execute/post_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl ExecuteHandler for execute::post_request::Execute {
let dr_id = self.posted_dr.try_hash()?;

// require the data request id to be unique
if state::data_request_or_result_exists(deps.as_ref(), dr_id) {
if state::data_request_exists(deps.as_ref(), dr_id) {
return Err(ContractError::DataRequestAlreadyExists);
}

Expand Down
3 changes: 0 additions & 3 deletions contract/src/msgs/data_requests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ impl QueryHandler for QueryMsg {
let reveals = dr.map(|dr| dr.reveals).unwrap_or_default();
to_json_binary(&reveals)?
}
QueryMsg::GetDataResult { dr_id } => {
to_json_binary(&state::may_load_result(deps.storage, &Hash::from_hex_str(&dr_id)?)?)?
}
QueryMsg::GetDataRequestsByStatus { status, offset, limit } => {
to_json_binary(&state::requests_by_status(deps.storage, &status, offset, limit)?)?
}
Expand Down
18 changes: 4 additions & 14 deletions contract/src/msgs/data_requests/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ use timeouts::Timeouts;
pub const TIMEOUT_CONFIG: Item<TimeoutConfig> = Item::new("timeout_config");

const DATA_REQUESTS: DataRequestsMap = new_enumerable_status_map!("data_request_pool");
const DATA_RESULTS: Map<&Hash, DataResult> = Map::new("data_results_pool");

pub fn init_data_requests(store: &mut dyn Storage) -> Result<(), ContractError> {
Ok(DATA_REQUESTS.initialize(store)?)
}

pub fn data_request_or_result_exists(deps: Deps, dr_id: Hash) -> bool {
DATA_REQUESTS.has(deps.storage, &dr_id) || DATA_RESULTS.has(deps.storage, &dr_id)
pub fn data_request_exists(deps: Deps, dr_id: Hash) -> bool {
DATA_REQUESTS.has(deps.storage, &dr_id)
}

pub fn may_load_request(store: &dyn Storage, dr_id: &Hash) -> StdResult<Option<DataRequest>> {
Expand Down Expand Up @@ -78,23 +77,14 @@ pub fn reveal(store: &mut dyn Storage, dr_id: Hash, dr: DataRequest, current_hei
Ok(())
}

pub fn post_result(store: &mut dyn Storage, dr_id: Hash, dr: &DataResult) -> StdResult<()> {
// we have to remove the request from the pool and save it to the results
DATA_RESULTS.save(store, &dr_id, dr)?;
pub fn post_result(store: &mut dyn Storage, dr_id: Hash) -> StdResult<()> {
// we have to remove the request from the pool
DATA_REQUESTS.remove(store, dr_id)?;
// no need to update status as we remove it from the requests pool

Ok(())
}

pub fn load_result(store: &dyn Storage, dr_id: &Hash) -> StdResult<DataResult> {
DATA_RESULTS.load(store, dr_id)
}

pub fn may_load_result(store: &dyn Storage, dr_id: &Hash) -> StdResult<Option<DataResult>> {
DATA_RESULTS.may_load(store, dr_id)
}

pub fn expire_data_requests(store: &mut dyn Storage, current_height: u64) -> StdResult<Vec<String>> {
DATA_REQUESTS.expire_data_requests(store, current_height)
}
Expand Down
15 changes: 6 additions & 9 deletions contract/src/msgs/data_requests/sudo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use super::{
};
pub(in crate::msgs::data_requests) mod expire_data_requests;
pub(in crate::msgs::data_requests) mod post_result;
pub(in crate::msgs::data_requests) mod post_results;
pub(in crate::msgs::data_requests) mod remove_requests;

fn post_result(result: sudo::PostResult, deps: &mut DepsMut, env: &Env) -> Result<Event, ContractError> {
fn post_result(result: sudo::RemoveDataRequest, deps: &mut DepsMut, env: &Env) -> Result<Event, ContractError> {
gluax marked this conversation as resolved.
Show resolved Hide resolved
// find the data request from the committed pool (if it exists, otherwise error)
let dr_id = Hash::from_hex_str(&result.dr_id)?;
let dr = state::load_request(deps.storage, &dr_id)?;
Expand All @@ -21,23 +21,20 @@ fn post_result(result: sudo::PostResult, deps: &mut DepsMut, env: &Env) -> Resul
("version", CONTRACT_VERSION.to_string()),
("dr_id", result.dr_id),
("block_height", block_height.to_string()),
("exit_code", result.exit_code.to_string()),
("result", to_json_string(&result.result)?),
("payback_address", dr.payback_address.to_base64()),
("seda_payload", dr.seda_payload.to_base64()),
]);

state::post_result(deps.storage, dr_id, &result.result)?;
state::post_result(deps.storage, dr_id)?;

let result_id = result.result.try_hash()?;
Ok(event.add_attribute("result_id", result_id.to_hex()))
Ok(event)
}

impl SudoHandler for SudoMsg {
fn sudo(self, deps: DepsMut, env: Env) -> Result<Response, ContractError> {
match self {
SudoMsg::PostDataResult(sudo) => sudo.sudo(deps, env),
SudoMsg::PostDataResults(sudo) => sudo.sudo(deps, env),
SudoMsg::RemoveDataRequest(sudo) => sudo.sudo(deps, env),
SudoMsg::RemoveDataRequests(sudo) => sudo.sudo(deps, env),
SudoMsg::ExpireDataRequests(sudo) => sudo.sudo(deps, env),
}
}
Expand Down
2 changes: 1 addition & 1 deletion contract/src/msgs/data_requests/sudo/post_result.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::*;

impl SudoHandler for sudo::PostResult {
impl SudoHandler for sudo::RemoveDataRequest {
/// Posts a data result to the contract
fn sudo(self, mut deps: DepsMut, env: Env) -> Result<Response, ContractError> {
let event = post_result(self, &mut deps, &env)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use super::*;

impl SudoHandler for sudo::post_results::Sudo {
impl SudoHandler for sudo::remove_requests::Sudo {
/// Posts data results to the contract
fn sudo(self, mut deps: DepsMut, env: Env) -> Result<Response, ContractError> {
let mut response = Response::new();
for event in self
.results
.requests
.into_iter()
.map(|result| post_result(result, &mut deps, &env))
{
Expand Down
43 changes: 6 additions & 37 deletions contract/src/msgs/data_requests/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,6 @@ pub fn construct_dr(dr_args: PostDataRequestArgs, seda_payload: Vec<u8>, height:
}
}

pub fn construct_result(dr: DataRequest, reveal: RevealBody, exit_code: u8) -> DataResult {
DataResult {
version: dr.version,
dr_id: dr.id,
block_height: dr.height,
exit_code,
gas_used: reveal.gas_used,
result: reveal.reveal,
payback_address: dr.payback_address,
seda_payload: dr.seda_payload,
consensus: true,
}
}

impl TestInfo {
#[track_caller]
pub fn post_data_request(
Expand Down Expand Up @@ -194,26 +180,17 @@ impl TestInfo {
}

#[track_caller]
pub fn post_data_result(&mut self, dr_id: String, result: DataResult, exit_code: u8) -> Result<(), ContractError> {
let msg = sudo::PostResult {
dr_id,
result,
exit_code,
}
.into();
pub fn post_data_result(&mut self, dr_id: String) -> Result<(), ContractError> {
gluax marked this conversation as resolved.
Show resolved Hide resolved
let msg = sudo::RemoveDataRequest { dr_id }.into();
self.sudo(&msg)
}

#[track_caller]
pub fn post_data_results(&mut self, results: Vec<(String, DataResult, u8)>) -> Result<(), ContractError> {
let msg = sudo::post_results::Sudo {
results: results
pub fn remove_data_requests(&mut self, results: Vec<String>) -> Result<(), ContractError> {
let msg = sudo::remove_requests::Sudo {
requests: results
.into_iter()
.map(|(dr_id, result, exit_code)| sudo::PostResult {
dr_id,
result,
exit_code,
})
.map(|dr_id| sudo::RemoveDataRequest { dr_id })
.collect(),
}
.into();
Expand All @@ -228,14 +205,6 @@ impl TestInfo {
.unwrap()
}

#[track_caller]
pub fn get_data_result(&self, dr_id: &str) -> Option<DataResult> {
self.query(query::QueryMsg::GetDataResult {
dr_id: dr_id.to_string(),
})
.unwrap()
}

#[track_caller]
pub fn get_data_request_commit(&self, dr_id: Hash, public_key: PublicKey) -> Option<Hash> {
self.query(query::QueryMsg::GetDataRequestCommitment {
Expand Down
78 changes: 7 additions & 71 deletions contract/src/msgs/data_requests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,12 +705,7 @@ fn post_data_result() {
test_info.reveal_result(&alice, &dr_id, alice_reveal.clone()).unwrap();

// owner posts a data result
let dr = test_info.get_data_request(&dr_id).unwrap();
let result = test_helpers::construct_result(dr, alice_reveal, 0);
test_info.post_data_result(dr_id.clone(), result, 0).unwrap();

// check we can get the results
let _res1 = test_info.get_data_result(&dr_id);
test_info.post_data_result(dr_id).unwrap();
}

#[test]
Expand Down Expand Up @@ -756,17 +751,7 @@ fn post_data_results() {
test_info.reveal_result(&alice, &dr_id2, alice_reveal2.clone()).unwrap();

// owner posts data results
let dr1 = test_info.get_data_request(&dr_id1).unwrap();
let result1 = test_helpers::construct_result(dr1, alice_reveal1, 0);
let dr2 = test_info.get_data_request(&dr_id2).unwrap();
let result2 = test_helpers::construct_result(dr2, alice_reveal2, 0);
test_info
.post_data_results(vec![(dr_id1.clone(), result1, 0), (dr_id2.clone(), result2, 0)])
.unwrap();

// check we can get the results
let _res1 = test_info.get_data_result(&dr_id1);
let _res2 = test_info.get_data_result(&dr_id2);
test_info.remove_data_requests(vec![dr_id1, dr_id2]).unwrap();
}

#[test]
Expand Down Expand Up @@ -812,9 +797,8 @@ fn cant_post_if_replication_factor_not_met() {
test_info.reveal_result(&alice, &dr_id, alice_reveal.clone()).unwrap();

// post a data result
let dr = test_info.get_data_request(&dr_id).unwrap();
let result = test_helpers::construct_result(dr, alice_reveal, 0);
test_info.post_data_result(dr_id, result, 0).unwrap();
test_info.get_data_request(&dr_id).unwrap();
test_info.post_data_result(dr_id).unwrap();
}

#[test]
Expand All @@ -840,42 +824,6 @@ fn check_data_request_id() {
assert_eq!(hex::encode(dr_id), expected_dr_id);
}

#[test]
fn check_data_result_id() {
// Expected RESULT ID for the following Data Result:
// {
// "version": "0.0.1",
// "dr_id": "74d7e8c9a77b7b4777153a32fcdf2424489f24cd59d3043eb2a30be7bba48306",
// "consensus": true,
// "exit_code": 0,
// "result": "Ghkvq84TmIuEmU1ClubNxBjVXi8df5QhiNQEC5T8V6w=",
// "block_height": 12345,
// "gas_used": 20,
// "payback_address": "",
// "seda_payload": ""
// }
let expected_result_id = "f6fc1b4ea295b00537bbe3e918793699c43dbc924ee7df650da567a095238150";
let dr_args = test_helpers::calculate_dr_id_and_args(0, 1);

// reveal sample
let alice_reveal = RevealBody {
id: expected_result_id.to_owned(),
salt: "123".into(),
reveal: "10".hash().into(),
gas_used: 20,
exit_code: 0,
proxy_public_keys: vec![],
};

// check if data result id matches expected value
let dr = test_helpers::construct_dr(dr_args, vec![0x04, 0x05, 0x06], 12345);
let result = test_helpers::construct_result(dr, alice_reveal, 0);

let result_id = result.try_hash().unwrap();

assert_eq!(hex::encode(result_id), expected_result_id);
}

#[test]
fn post_data_result_with_more_drs_in_the_pool() {
let mut test_info = TestInfo::init();
Expand Down Expand Up @@ -941,8 +889,7 @@ fn post_data_result_with_more_drs_in_the_pool() {
// Post only first dr ready to be tallied (while there is another one in the pool and not ready)
// This checks part of the swap_remove logic
let dr = dr_to_be_tallied[0].clone();
let result1 = test_helpers::construct_result(dr.clone(), alice_reveal.clone(), 0);
test_info.post_data_result(dr.id, result1, 0).unwrap();
test_info.post_data_result(dr.id).unwrap();
assert_eq!(
0,
test_info
Expand All @@ -957,8 +904,7 @@ fn post_data_result_with_more_drs_in_the_pool() {

// Post last dr result
let dr = dr_to_be_tallied[0].clone();
let result1 = test_helpers::construct_result(dr.clone(), alice_reveal, 0);
test_info.post_data_result(dr.id, result1, 0).unwrap();
test_info.post_data_result(dr.id).unwrap();

// Check dr to be tallied is empty
assert_eq!(
Expand Down Expand Up @@ -1119,17 +1065,7 @@ fn get_data_requests_by_status_with_many_more_drs_in_pool() {
.enumerate()
{
if i % 8 == 0 {
let alice_reveal = RevealBody {
id: request.id.clone(),
salt: alice.salt(),
reveal: "10".hash().into(),
gas_used: 0,
exit_code: 0,
proxy_public_keys: vec![],
};
let dr_info = test_info.get_data_request(&request.id).unwrap();
let result = test_helpers::construct_result(dr_info.clone(), alice_reveal.clone(), 0);
test_info.post_data_result(request.id.to_string(), result, 0).unwrap();
test_info.post_data_result(request.id.to_string()).unwrap();
}
}
assert_eq!(
Expand Down
5 changes: 1 addition & 4 deletions contract/src/msgs/staking/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use data_requests::test::test_helpers::construct_result;
use msgs::data_requests::RevealBody;
use seda_common::msgs::staking::{Staker, StakingConfig};

Expand Down Expand Up @@ -255,9 +254,7 @@ fn executor_not_eligible_if_dr_resolved() {
test_info.reveal_result(&anyone, &dr_id, reveal.clone()).unwrap();

// Owner posts the result
let dr = test_info.get_data_request(&dr_id).unwrap();
let result = construct_result(dr, reveal, 0);
test_info.post_data_result(dr_id.clone(), result, 0).unwrap();
test_info.post_data_result(dr_id.clone()).unwrap();

// perform the check
let is_executor_eligible = test_info.is_executor_eligible(&anyone, dr_id);
Expand Down