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

System entry point #793

Merged
merged 8 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ and this project adheres to
different implementation) ([#692], [#711], [#714])
- cosmwasm-std: Added new `WasmMsg::Migrate` variant that allows one contract
(eg. multisig) be the admin and migrate another contract ([#768])
- cosmwasm-std: Added optional `system` entry point that can only be called by
native (blockchain) modules to expose admin functionality if desired. ([#793])

[#692]: https://github.com/CosmWasm/cosmwasm/issues/692
[#706]: https://github.com/CosmWasm/cosmwasm/pull/706
Expand All @@ -47,6 +49,7 @@ and this project adheres to
[#714]: https://github.com/CosmWasm/cosmwasm/pull/714
[#716]: https://github.com/CosmWasm/cosmwasm/pull/716
[#768]: https://github.com/CosmWasm/cosmwasm/pull/768
[#793]: https://github.com/CosmWasm/cosmwasm/pull/793

### Changed

Expand Down
5 changes: 4 additions & 1 deletion contracts/hackatom/examples/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use std::fs::create_dir_all;
use cosmwasm_schema::{export_schema, remove_schemas, schema_for};
use cosmwasm_std::BalanceResponse;

use hackatom::contract::{HandleMsg, InitMsg, MigrateMsg, QueryMsg, State, VerifierResponse};
use hackatom::contract::{
HandleMsg, InitMsg, MigrateMsg, QueryMsg, State, SystemMsg, VerifierResponse,
};

fn main() {
let mut out_dir = current_dir().unwrap();
Expand All @@ -15,6 +17,7 @@ fn main() {
export_schema(&schema_for!(InitMsg), &out_dir);
export_schema(&schema_for!(HandleMsg), &out_dir);
export_schema(&schema_for!(MigrateMsg), &out_dir);
export_schema(&schema_for!(SystemMsg), &out_dir);
export_schema(&schema_for!(QueryMsg), &out_dir);
export_schema(&schema_for!(State), &out_dir);
export_schema(&schema_for!(VerifierResponse), &out_dir);
Expand Down
44 changes: 44 additions & 0 deletions contracts/hackatom/schema/system_msg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "SystemMsg",
"description": "SystemMsg is only expose for internal sdk modules to call. This is showing how we can expose \"admin\" functionality than can not be called by external users or contracts, but only trusted (native/Go) code in the blockchain",
"type": "object",
"required": [
"amount",
"recipient"
],
"properties": {
"amount": {
"type": "array",
"items": {
"$ref": "#/definitions/Coin"
}
},
"recipient": {
"$ref": "#/definitions/HumanAddr"
}
},
"definitions": {
"Coin": {
"type": "object",
"required": [
"amount",
"denom"
],
"properties": {
"amount": {
"$ref": "#/definitions/Uint128"
},
"denom": {
"type": "string"
}
}
},
"HumanAddr": {
"type": "string"
},
"Uint128": {
"type": "string"
}
}
}
59 changes: 56 additions & 3 deletions contracts/hackatom/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};

use cosmwasm_std::{
from_slice, to_binary, to_vec, AllBalanceResponse, Api, BankMsg, Binary, CanonicalAddr, Deps,
DepsMut, Env, HumanAddr, MessageInfo, QueryRequest, QueryResponse, Response, StdError,
StdResult, WasmQuery,
entry_point, from_slice, to_binary, to_vec, AllBalanceResponse, Api, BankMsg, Binary,
CanonicalAddr, Coin, Deps, DepsMut, Env, HumanAddr, MessageInfo, QueryRequest, QueryResponse,
Response, StdError, StdResult, WasmQuery,
};

use crate::errors::HackError;
Expand All @@ -30,6 +30,15 @@ pub struct MigrateMsg {
pub verifier: HumanAddr,
}

/// SystemMsg is only expose for internal sdk modules to call.
/// This is showing how we can expose "admin" functionality than can not be called by
/// external users or contracts, but only trusted (native/Go) code in the blockchain
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct SystemMsg {
pub recipient: HumanAddr,
pub amount: Vec<Coin>,
}
Copy link
Member

Choose a reason for hiding this comment

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

What about structuring this as an enum, such that this gets a verb like SystemMsg::StealFunds as well as extensibility.

Copy link
Member Author

@ethanfrey ethanfrey Feb 24, 2021

Choose a reason for hiding this comment

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

As an example sure.

I am happy to rename it, but it is designed to allow some priviledged "system" calls as opposed to external calls. Unless you have a better name, I would like to merge and then we can rename before the 0.14 release when we have a good name.

Copy link
Member

@webmaster128 webmaster128 Feb 24, 2021

Choose a reason for hiding this comment

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

As an example sure.

Yeah, I think it is easier for readers to habve a one-case enum than trying to wrap their head around a struct/enum mix.

I am happy to rename it, but it is designed to allow some priviledged "system" calls as opposed to external calls. Unless you have a better name, I would like to merge and then we can rename before the 0.14 release when we have a good name.

Agreed, let's merge as it. Will think about it.

Copy link
Contributor

@maurolacy maurolacy Feb 24, 2021

Choose a reason for hiding this comment

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

Sorry, I missed this. Some late comments.

Regarding naming:
Maybe indicating the priviledged nature of the call in the name would be a good idea. Priviledged sounds bad, but maybe Admin, Administer, or even Root are good alternatives to System.

System / Admin for the messages / enums, and administer / root as a function might work.

Copy link
Member Author

Choose a reason for hiding this comment

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

So we can root the contracts? I like it 😄

Copy link
Member

Choose a reason for hiding this comment

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

oO bad things showing up in the dictionary.

But looking the root/admin direction, what about sudo. It is short, and a verb.

Copy link
Contributor

@maurolacy maurolacy Feb 25, 2021

Choose a reason for hiding this comment

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

I like it. It means "superuser do", which gives us another option: superuser. But I think sudo is better, as it captures the action.


#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct State {
pub verifier: CanonicalAddr,
Expand Down Expand Up @@ -122,6 +131,17 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, Ha
Ok(Response::default())
}

#[entry_point]
pub fn system(_deps: DepsMut, _env: Env, msg: SystemMsg) -> Result<Response, HackError> {
let msg = BankMsg::Send {
to_address: msg.recipient,
amount: msg.amount,
};
let mut response = Response::default();
response.add_message(msg);
Ok(response)
}

pub fn handle(
deps: DepsMut,
env: Env,
Expand Down Expand Up @@ -433,6 +453,39 @@ mod tests {
assert_eq!(query_response.verifier, new_verifier);
}

#[test]
fn system_can_steal_tokens() {
let mut deps = mock_dependencies(&[]);

let verifier = HumanAddr::from("verifies");
let beneficiary = HumanAddr::from("benefits");
let creator = HumanAddr::from("creator");
let msg = InitMsg {
verifier: verifier.clone(),
beneficiary,
};
let info = mock_info(creator.as_str(), &[]);
let res = init(deps.as_mut(), mock_env(), info, msg).unwrap();
assert_eq!(0, res.messages.len());

// system takes any tax it wants
let sys_msg = SystemMsg {
recipient: "community-pool".into(),
amount: coins(700, "gold"),
};
let res = system(deps.as_mut(), mock_env(), sys_msg.clone()).unwrap();
assert_eq!(1, res.messages.len());
let msg = res.messages.get(0).expect("no message");
assert_eq!(
msg,
&BankMsg::Send {
to_address: sys_msg.recipient,
amount: sys_msg.amount,
}
.into(),
);
}

#[test]
fn querier_callbacks_work() {
let rich_addr = HumanAddr::from("foobar");
Expand Down
37 changes: 35 additions & 2 deletions contracts/hackatom/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ use cosmwasm_vm::{
call_handle, from_slice,
testing::{
handle, init, migrate, mock_env, mock_info, mock_instance, mock_instance_with_balances,
query, test_io, MOCK_CONTRACT_ADDR,
query, system, test_io, MOCK_CONTRACT_ADDR,
},
BackendApi, Storage, VmError,
};

use hackatom::contract::{HandleMsg, InitMsg, MigrateMsg, QueryMsg, State, CONFIG_KEY};
use hackatom::contract::{HandleMsg, InitMsg, MigrateMsg, QueryMsg, State, SystemMsg, CONFIG_KEY};

static WASM: &[u8] = include_bytes!("../target/wasm32-unknown-unknown/release/hackatom.wasm");

Expand Down Expand Up @@ -145,6 +145,39 @@ fn migrate_verifier() {
);
}

#[test]
fn system_can_steal_tokens() {
let mut deps = mock_instance(WASM, &[]);

let verifier = HumanAddr::from("verifies");
let beneficiary = HumanAddr::from("benefits");
let creator = HumanAddr::from("creator");
let msg = InitMsg {
verifier: verifier.clone(),
beneficiary,
};
let info = mock_info(creator.as_str(), &[]);
let res: Response = init(&mut deps, mock_env(), info, msg).unwrap();
assert_eq!(0, res.messages.len());

// system takes any tax it wants
let sys_msg = SystemMsg {
recipient: "community-pool".into(),
amount: coins(700, "gold"),
};
let res: Response = system(&mut deps, mock_env(), sys_msg.clone()).unwrap();
assert_eq!(1, res.messages.len());
let msg = res.messages.get(0).expect("no message");
assert_eq!(
msg,
&BankMsg::Send {
to_address: sys_msg.recipient,
amount: sys_msg.amount,
}
.into(),
);
}

#[test]
fn querier_callbacks_work() {
let rich_addr = HumanAddr::from("foobar");
Expand Down
40 changes: 40 additions & 0 deletions packages/std/src/exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,26 @@ where
release_buffer(v) as u32
}

/// do_system should be wrapped in an external "C" export, containing a contract-specific function as arg
///
/// - `M`: message type for request
/// - `C`: custom response message type (see CosmosMsg)
/// - `E`: error type for responses
pub fn do_system<M, C, E>(
system_fn: &dyn Fn(DepsMut, Env, M) -> Result<Response<C>, E>,
env_ptr: u32,
msg_ptr: u32,
) -> u32
where
M: DeserializeOwned + JsonSchema,
C: Serialize + Clone + fmt::Debug + PartialEq + JsonSchema,
E: ToString,
{
let res = _do_system(system_fn, env_ptr as *mut Region, msg_ptr as *mut Region);
let v = to_vec(&res).unwrap();
release_buffer(v) as u32
}

/// do_query should be wrapped in an external "C" export, containing a contract-specific function as arg
///
/// - `M`: message type for request
Expand Down Expand Up @@ -220,6 +240,26 @@ where
migrate_fn(deps.as_mut(), env, msg).into()
}

fn _do_system<M, C, E>(
system_fn: &dyn Fn(DepsMut, Env, M) -> Result<Response<C>, E>,
env_ptr: *mut Region,
msg_ptr: *mut Region,
) -> ContractResult<Response<C>>
where
M: DeserializeOwned + JsonSchema,
C: Serialize + Clone + fmt::Debug + PartialEq + JsonSchema,
E: ToString,
{
let env: Vec<u8> = unsafe { consume_region(env_ptr) };
let msg: Vec<u8> = unsafe { consume_region(msg_ptr) };

let env: Env = try_into_contract_result!(from_slice(&env));
let msg: M = try_into_contract_result!(from_slice(&msg));

let mut deps = make_dependencies();
system_fn(deps.as_mut(), env, msg).into()
}

fn _do_query<M, E>(
query_fn: &dyn Fn(Deps, Env, M) -> Result<QueryResponse, E>,
env_ptr: *mut Region,
Expand Down
2 changes: 1 addition & 1 deletion packages/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ mod imports;
mod memory; // Used by exports and imports only. This assumes pointers are 32 bit long, which makes it untestable on dev machines.

#[cfg(target_arch = "wasm32")]
pub use crate::exports::{do_handle, do_init, do_migrate, do_query};
pub use crate::exports::{do_handle, do_init, do_migrate, do_query, do_system};
#[cfg(target_arch = "wasm32")]
pub use crate::imports::{ExternalApi, ExternalQuerier, ExternalStorage};

Expand Down
34 changes: 34 additions & 0 deletions packages/vm/src/calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::serde::{from_slice, to_vec};
const MAX_LENGTH_INIT: usize = 100_000;
const MAX_LENGTH_HANDLE: usize = 100_000;
const MAX_LENGTH_MIGRATE: usize = 100_000;
const MAX_LENGTH_SYSTEM: usize = 100_000;
const MAX_LENGTH_QUERY: usize = 100_000;

pub fn call_init<A, S, Q, U>(
Expand Down Expand Up @@ -71,6 +72,23 @@ where
Ok(result)
}

pub fn call_system<A, S, Q, U>(
instance: &mut Instance<A, S, Q>,
env: &Env,
msg: &[u8],
) -> VmResult<ContractResult<Response<U>>>
where
A: BackendApi + 'static,
S: Storage + 'static,
Q: Querier + 'static,
U: DeserializeOwned + Clone + fmt::Debug + JsonSchema + PartialEq,
{
let env = to_vec(env)?;
let data = call_system_raw(instance, &env, msg)?;
let result: ContractResult<Response<U>> = from_slice(&data)?;
Ok(result)
}

pub fn call_query<A, S, Q>(
instance: &mut Instance<A, S, Q>,
env: &Env,
Expand Down Expand Up @@ -144,6 +162,22 @@ where
call_raw(instance, "migrate", &[env, msg], MAX_LENGTH_MIGRATE)
}

/// Calls Wasm export "system" and returns raw data from the contract.
/// The result is length limited to prevent abuse but otherwise unchecked.
pub fn call_system_raw<A, S, Q>(
instance: &mut Instance<A, S, Q>,
env: &[u8],
msg: &[u8],
) -> VmResult<Vec<u8>>
where
A: BackendApi + 'static,
S: Storage + 'static,
Q: Querier + 'static,
{
instance.set_storage_readonly(false);
call_raw(instance, "system", &[env, msg], MAX_LENGTH_SYSTEM)
}

/// Calls Wasm export "query" and returns raw data from the contract.
/// The result is length limited to prevent abuse but otherwise unchecked.
pub fn call_query_raw<A, S, Q>(
Expand Down
2 changes: 1 addition & 1 deletion packages/vm/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ mod singlepass_tests {
.unwrap();

let handle_used = gas_before_handle - instance.get_gas_left();
assert_eq!(handle_used, 165757);
assert_eq!(handle_used, 165794);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion packages/vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub use crate::backend::{
pub use crate::cache::{AnalysisReport, Cache, CacheOptions, Stats};
pub use crate::calls::{
call_handle, call_handle_raw, call_init, call_init_raw, call_migrate, call_migrate_raw,
call_query, call_query_raw,
call_query, call_query_raw, call_system, call_system_raw,
};
pub use crate::checksum::Checksum;

Expand Down
2 changes: 1 addition & 1 deletion packages/vm/src/static_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ mod tests {
false
}
});
assert_eq!(exported_functions.count(), 7); // 6 required export plus "migrate"
assert_eq!(exported_functions.count(), 8); // 6 required export plus "migrate" and "system"

let exported_memories = module
.export_section()
Expand Down
Loading