From 611e8d74ac3aa8d2cb886ca005069a85ace818ce Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 6 Nov 2022 18:33:28 +0800 Subject: [PATCH 01/10] Add payroll example contract --- workspaces-tests/Cargo.toml | 3 + workspaces-tests/src/bin/payroll_example.rs | 109 ++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 workspaces-tests/src/bin/payroll_example.rs diff --git a/workspaces-tests/Cargo.toml b/workspaces-tests/Cargo.toml index 8665e4c..4031f4c 100644 --- a/workspaces-tests/Cargo.toml +++ b/workspaces-tests/Cargo.toml @@ -29,6 +29,9 @@ name = "native_multisig" [[bin]] name = "simple_multisig" +[[bin]] +name = "payroll_example" + [dependencies] near-contract-tools = {path = "../"} near-sdk = {version = "4.0.0", features = ["unstable"]} diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs new file mode 100644 index 0000000..34de7a7 --- /dev/null +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -0,0 +1,109 @@ +use near_contract_tools::{ + rbac::{self, Rbac}, + Rbac, +}; +use near_sdk::{ + borsh::{self, BorshDeserialize, BorshSerialize}, + collections::UnorderedMap, + env, ext_contract, near_bindgen, require, AccountId, BorshStorageKey, IntoStorageKey, + PanicOnDefault, Promise, +}; + +enum PayrollKey { + LOG, + FEE, +} + +impl IntoStorageKey for PayrollKey { + fn into_storage_key(self) -> Vec { + match self { + PayrollKey::LOG => b"~pl".to_vec(), + PayrollKey::FEE => b"~pf".to_vec(), + } + } +} + +#[derive(BorshSerialize, BorshDeserialize, BorshStorageKey)] +enum Role { + Manager, + Employee, +} + +#[derive(BorshSerialize, BorshDeserialize, PanicOnDefault, Rbac)] +#[rbac(roles = "Role")] +#[near_bindgen] +struct Payroll { + pub hourly_fee: u32, + pub logged_time: UnorderedMap, +} + +/// Manager can add new employees and disburse payments +/// Employees can log time +#[near_bindgen] +impl Payroll { + pub fn new() -> Self { + Payroll { + hourly_fee: 1000, + logged_time: UnorderedMap::new(PayrollKey::LOG), + } + } + + /// Manager can add new employees + pub fn add_employee(&mut self, account_id: &AccountId) { + self.require_role(&Role::Manager); + self.logged_time.insert(account_id, &0); + + // write updated time log to state + env::state_write(&self.logged_time); + } + + /// Manager can start payment + pub fn disburse_pay(&self) { + self.require_role(&Role::Manager); + self.logged_time + .iter() + .for_each(|(account_id, logged_time)| { + Promise::new(account_id).transfer(logged_time as u128 * self.hourly_fee as u128); + }); + } + + /// Employee can log time + pub fn log_time(&mut self, hours: u8) { + self.require_role(&Role::Employee); + let employee_id = env::predecessor_account_id(); + let current_hours = self.logged_time.get(&employee_id).unwrap_or_else(|| { + env::panic_str(format!("No employee exists for account: {}", employee_id).as_str()) + }); + + // Add entry for employee's account id + self.logged_time + .insert(&employee_id, &(current_hours + hours)); + + // write updated time log to state + env::state_write(&self.logged_time); + } +} + +/// External methods for payroll +#[ext_contract(ext_payroll)] +pub trait PayrollExternal { + fn payroll_add_employee(&mut self, account_id: AccountId); + fn disburse_pay(&self); + fn log_time(&mut self, hours: u8); +} + +impl PayrollExternal for Payroll { + fn payroll_add_employee(&mut self, account_id: AccountId) { + self.add_employee(&account_id); + } + + fn disburse_pay(&self) { + self.disburse_pay(); + } + + fn log_time(&mut self, hours: u8) { + self.log_time(hours); + } +} + +pub fn main() {} From 993e4bc9ffa5494cd5294e5480fa80d08f1d741c Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Thu, 10 Nov 2022 19:45:19 +0800 Subject: [PATCH 02/10] Fix state writing and use pull style for payments --- workspaces-tests/src/bin/payroll_example.rs | 43 +++++++++++---------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index 34de7a7..7267a07 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -1,7 +1,5 @@ -use near_contract_tools::{ - rbac::{self, Rbac}, - Rbac, -}; +//! Payroll system manages employee and their pay +use near_contract_tools::{rbac::Rbac, Rbac}; use near_sdk::{ borsh::{self, BorshDeserialize, BorshSerialize}, collections::UnorderedMap, @@ -54,17 +52,17 @@ impl Payroll { self.logged_time.insert(account_id, &0); // write updated time log to state - env::state_write(&self.logged_time); + env::state_write(self); } - /// Manager can start payment - pub fn disburse_pay(&self) { - self.require_role(&Role::Manager); - self.logged_time - .iter() - .for_each(|(account_id, logged_time)| { - Promise::new(account_id).transfer(logged_time as u128 * self.hourly_fee as u128); - }); + /// Employee can request pay + pub fn request_pay(&self) -> Promise { + self.require_role(&Role::Employee); + let employee_id = env::predecessor_account_id(); + let logged_time = self.logged_time.get(&employee_id).unwrap_or_else(|| { + env::panic_str(format!("No employee exists for account: {}", employee_id).as_str()) + }); + Promise::new(employee_id).transfer(logged_time as u128 * self.hourly_fee as u128) } /// Employee can log time @@ -80,16 +78,19 @@ impl Payroll { .insert(&employee_id, &(current_hours + hours)); // write updated time log to state - env::state_write(&self.logged_time); + env::state_write(self); } } -/// External methods for payroll #[ext_contract(ext_payroll)] +/// External methods for payroll pub trait PayrollExternal { + /// Manager can add new employees fn payroll_add_employee(&mut self, account_id: AccountId); - fn disburse_pay(&self); - fn log_time(&mut self, hours: u8); + /// Employee can log time + fn payroll_log_time(&mut self, hours: u8); + /// Employee can request for pay + fn payroll_request_pay(&self) -> Promise; } impl PayrollExternal for Payroll { @@ -97,12 +98,12 @@ impl PayrollExternal for Payroll { self.add_employee(&account_id); } - fn disburse_pay(&self) { - self.disburse_pay(); + fn payroll_log_time(&mut self, hours: u8) { + self.log_time(hours); } - fn log_time(&mut self, hours: u8) { - self.log_time(hours); + fn payroll_request_pay(&self) -> Promise { + self.request_pay() } } From b8b7307d00e4f62cb26d7f9a9b2769b031b407ad Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Wed, 30 Nov 2022 19:23:31 +0800 Subject: [PATCH 03/10] Add approval management for payroll Add documentation/design comments in TODO notes --- src/approval/mod.rs | 39 ++++++ workspaces-tests/Cargo.toml | 1 + workspaces-tests/src/bin/payroll_example.rs | 147 ++++++++++++++++++-- 3 files changed, 174 insertions(+), 13 deletions(-) diff --git a/src/approval/mod.rs b/src/approval/mod.rs index 0c49c79..1440d7f 100644 --- a/src/approval/mod.rs +++ b/src/approval/mod.rs @@ -27,6 +27,19 @@ pub trait Action { /// Defines the operating parameters for an ApprovalManager and performs /// approvals +/// +/// TODO: unclear if the approval configuration should be defined on the contract +/// a special configuration struct (as in the case multi-sig) or for a simple +/// approval the state S itself. +/// +/// TODO: Explain that the approval configuration parameters A and S are +/// the Action and Action state (based on which the action is approved) +/// +/// TODO: Good to have sane defaults for error types but blocked on issue +/// https://github.com/rust-lang/rust/issues/29661. Mention sane defaults +/// and choice behind them in documentation. +/// +/// TODO: in many cases the state is the configuration pub trait ApprovalConfiguration { /// Errors when approving a request type ApprovalError; @@ -53,6 +66,8 @@ pub trait ApprovalConfiguration { action_request: &ActionRequest, ) -> Result<(), Self::AuthorizationError>; + /// TODO: can pass &AccountId here as well to be consistent with + /// is_account_authorized function /// Modify action_request.approval_state in-place to increase approval fn try_approve_with_authorized_account( &self, @@ -127,6 +142,20 @@ pub enum RemovalError { /// Collection of action requests that manages their approval state and /// execution +/// +/// TODO: Explain what the module does that A, S, C refer to the types +/// for the action, the approvals state for the action and the +/// type that approves an action based on it's state +/// +/// TODO: Mention that this trait is only for creating, managing, removing +/// approvals. The defaults are sane and it can just be implemented as +/// is for any contract without any changes to the methods unless needed. +/// +/// TODO: I would expect the approval manager to automatically execute +/// the request once the required condition is met. This can be considered +/// for a default setting in the approval manager. If this is not the +/// case then it can be documented that the approvals and executions are +/// entirely decided by the contract. pub trait ApprovalManager where A: Action + BorshSerialize + BorshDeserialize, @@ -228,6 +257,16 @@ where Ok(result) } + /// TODO: a name like `request_is_approved` or `is_request_approved` will + /// be more consistent with the rest of the function names and hence + /// easier to find + /// + /// TODO: Moreover this clashes with the function of the same name in + /// ApprovalConfiguration so a struct that implements both Configuration + /// and Manager traits cannot choose which one to call. This again can + /// be fixed by renaming this one and adding some documentation about + /// which one to use and when. + /// /// Is the given request ID able to be executed if such a request were to /// be initiated by an authorized account? fn is_approved_for_execution(request_id: u32) -> Result<(), C::ExecutionEligibilityError> { diff --git a/workspaces-tests/Cargo.toml b/workspaces-tests/Cargo.toml index 4031f4c..aa1ca7b 100644 --- a/workspaces-tests/Cargo.toml +++ b/workspaces-tests/Cargo.toml @@ -35,6 +35,7 @@ name = "payroll_example" [dependencies] near-contract-tools = {path = "../"} near-sdk = {version = "4.0.0", features = ["unstable"]} +serde = "1.0.148" strum = "0.24.1" strum_macros = "0.24.3" thiserror = "1.0.34" diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index 7267a07..09a34e6 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -1,26 +1,117 @@ //! Payroll system manages employee and their pay -use near_contract_tools::{rbac::Rbac, Rbac}; +use near_contract_tools::{ + approval::{self, ApprovalConfiguration, ApprovalManager}, + rbac::Rbac, + Rbac, +}; use near_sdk::{ borsh::{self, BorshDeserialize, BorshSerialize}, collections::UnorderedMap, - env, ext_contract, near_bindgen, require, AccountId, BorshStorageKey, IntoStorageKey, - PanicOnDefault, Promise, + env, ext_contract, near_bindgen, + serde::Serialize, + AccountId, BorshStorageKey, PanicOnDefault, Promise, }; +#[derive(BorshStorageKey, BorshSerialize)] enum PayrollKey { LOG, - FEE, } -impl IntoStorageKey for PayrollKey { - fn into_storage_key(self) -> Vec { - match self { - PayrollKey::LOG => b"~pl".to_vec(), - PayrollKey::FEE => b"~pf".to_vec(), +#[derive(BorshSerialize, BorshDeserialize)] +struct PayrollAction(AccountId, u8); + +impl approval::Action for PayrollAction { + type Output = Promise; + + fn execute(self, contract: &mut Payroll) -> Self::Output { + let PayrollAction(employee_id, hours) = self; + Promise::new(employee_id).transfer(hours as u128 * contract.hourly_fee as u128) + } +} + +/// Both manager and employee need to approve payment request +#[derive(BorshSerialize, BorshDeserialize, Serialize)] +pub enum PayrollApproval { + EmployeeApproved, + ManagerApproved, + BothApproved, +} + +impl ApprovalConfiguration for Payroll { + type ApprovalError = String; + type RemovalError = (); + type AuthorizationError = String; + type ExecutionEligibilityError = String; + + fn is_approved_for_execution( + &self, + action_request: &approval::ActionRequest, + ) -> Result<(), Self::ExecutionEligibilityError> { + match action_request.approval_state { + PayrollApproval::EmployeeApproved => Err("Manager has not approved yet".to_string()), + PayrollApproval::ManagerApproved => Err("Employee has not accepted yet".to_string()), + PayrollApproval::BothApproved => Ok(()), + } + } + + fn is_removable( + &self, + action_request: &approval::ActionRequest, + ) -> Result<(), Self::RemovalError> { + Ok(()) + } + + fn is_account_authorized( + &self, + account_id: &AccountId, + action_request: &approval::ActionRequest, + ) -> Result<(), Self::AuthorizationError> { + match ( + ::has_role(account_id, &Role::Manager), + action_request.action.0.eq(account_id), + ) { + (true, true) => Err("An employee cannot be their own manager".to_string()), + (true, false) => Ok(()), + (false, true) => Ok(()), + (false, false) => Err("Unauthorized account".to_string()), + } + } + + fn try_approve_with_authorized_account( + &self, + account_id: AccountId, + action_request: &mut approval::ActionRequest, + ) -> Result<(), Self::ApprovalError> { + match ( + ::has_role(&account_id, &Role::Manager), + action_request.action.0 == account_id, + ) { + (true, true) => Err("An employee cannot be their own manager".to_string()), + (true, false) => { + match action_request.approval_state { + PayrollApproval::EmployeeApproved => { + action_request.approval_state = PayrollApproval::BothApproved + } + _ => (), + } + Ok(()) + } + (false, true) => { + match action_request.approval_state { + PayrollApproval::ManagerApproved => { + action_request.approval_state = PayrollApproval::BothApproved + } + _ => (), + } + Ok(()) + } + (false, false) => Err("Unauthorized account".to_string()), } } } +impl ApprovalManager for Payroll {} + #[derive(BorshSerialize, BorshDeserialize, BorshStorageKey)] enum Role { Manager, @@ -55,14 +146,32 @@ impl Payroll { env::state_write(self); } + pub fn approve_pay(&mut self, request_id: u32) { + self.approve_request(request_id).unwrap(); + } + + pub fn get_pay(&mut self, request_id: u32) -> Promise { + self.execute_request(request_id).unwrap() + } + /// Employee can request pay - pub fn request_pay(&self) -> Promise { + pub fn request_pay(&mut self) -> u32 { self.require_role(&Role::Employee); let employee_id = env::predecessor_account_id(); let logged_time = self.logged_time.get(&employee_id).unwrap_or_else(|| { env::panic_str(format!("No employee exists for account: {}", employee_id).as_str()) }); - Promise::new(employee_id).transfer(logged_time as u128 * self.hourly_fee as u128) + + let request_id = self + .create_request( + PayrollAction(employee_id, logged_time), + PayrollApproval::EmployeeApproved, + ) + .unwrap(); + + near_sdk::log!(format!("Request ID: {request_id}")); + + request_id } /// Employee can log time @@ -90,7 +199,11 @@ pub trait PayrollExternal { /// Employee can log time fn payroll_log_time(&mut self, hours: u8); /// Employee can request for pay - fn payroll_request_pay(&self) -> Promise; + fn payroll_request_pay(&mut self) -> u32; + /// Pay can be approved by manager and the employee who made the request + fn payroll_approve_pay(&mut self, request_id: u32); + /// If request is approved get_pay will return a promise to transfer funds + fn payroll_get_pay(&mut self, request_id: u32) -> Promise; } impl PayrollExternal for Payroll { @@ -102,9 +215,17 @@ impl PayrollExternal for Payroll { self.log_time(hours); } - fn payroll_request_pay(&self) -> Promise { + fn payroll_request_pay(&mut self) -> u32 { self.request_pay() } + + fn payroll_approve_pay(&mut self, request_id: u32) { + self.approve_pay(request_id); + } + + fn payroll_get_pay(&mut self, request_id: u32) -> Promise { + self.get_pay(request_id) + } } pub fn main() {} From 7e8ae1ca814aedb4b7a8c188c054f4dde7ceccaf Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 4 Dec 2022 20:05:14 +0800 Subject: [PATCH 04/10] Add method --- workspaces-tests/src/bin/payroll_example.rs | 44 +++++++++++++++------ 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index 09a34e6..fef018b 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -126,24 +126,43 @@ struct Payroll { pub logged_time: UnorderedMap, } +/// Approval state for making payment +#[derive(BorshSerialize, BorshDeserialize, Debug)] +enum PayApprovalState { + EmployeeApproved, + ManagerApproved, + BothApproved, +} + /// Manager can add new employees and disburse payments /// Employees can log time +/// +/// Note: After any &mut self function the contract state is automatically +/// written back. An explicit env::state_write call is not needed. +/// +/// https://docs.near.org/sdk/rust/contract-structure/near-bindgen #[near_bindgen] impl Payroll { - pub fn new() -> Self { - Payroll { + #[init] + pub fn new(owner: &AccountId) -> Self { + let mut contract = Payroll { hourly_fee: 1000, logged_time: UnorderedMap::new(PayrollKey::LOG), - } + }; + contract.add_role(owner, &Role::Manager); + contract + } + + /// Manager can add new managers + pub fn add_manager(&mut self, account_id: &AccountId) { + self.require_role(&Role::Manager); + self.add_role(account_id, &Role::Manager); } /// Manager can add new employees pub fn add_employee(&mut self, account_id: &AccountId) { self.require_role(&Role::Manager); self.logged_time.insert(account_id, &0); - - // write updated time log to state - env::state_write(self); } pub fn approve_pay(&mut self, request_id: u32) { @@ -159,7 +178,7 @@ impl Payroll { self.require_role(&Role::Employee); let employee_id = env::predecessor_account_id(); let logged_time = self.logged_time.get(&employee_id).unwrap_or_else(|| { - env::panic_str(format!("No employee exists for account: {}", employee_id).as_str()) + env::panic_str(format!("No record exists for account: {}", employee_id).as_str()) }); let request_id = self @@ -179,21 +198,20 @@ impl Payroll { self.require_role(&Role::Employee); let employee_id = env::predecessor_account_id(); let current_hours = self.logged_time.get(&employee_id).unwrap_or_else(|| { - env::panic_str(format!("No employee exists for account: {}", employee_id).as_str()) + env::panic_str(format!("No record exists for account: {}", employee_id).as_str()) }); // Add entry for employee's account id self.logged_time .insert(&employee_id, &(current_hours + hours)); - - // write updated time log to state - env::state_write(self); } } #[ext_contract(ext_payroll)] /// External methods for payroll pub trait PayrollExternal { + /// Manager can add new managers + fn payroll_add_manager(&mut self, account_id: &AccountId); /// Manager can add new employees fn payroll_add_employee(&mut self, account_id: AccountId); /// Employee can log time @@ -207,6 +225,10 @@ pub trait PayrollExternal { } impl PayrollExternal for Payroll { + fn payroll_add_manager(&mut self, account_id: &AccountId) { + self.add_manager(account_id); + } + fn payroll_add_employee(&mut self, account_id: AccountId) { self.add_employee(&account_id); } From b2741915b88ce5e8b52a646e3494cb986392f86a Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 4 Dec 2022 20:05:30 +0800 Subject: [PATCH 05/10] Add payroll sandbox test setup --- workspaces-tests/tests/payroll.rs | 74 +++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 workspaces-tests/tests/payroll.rs diff --git a/workspaces-tests/tests/payroll.rs b/workspaces-tests/tests/payroll.rs new file mode 100644 index 0000000..ecada99 --- /dev/null +++ b/workspaces-tests/tests/payroll.rs @@ -0,0 +1,74 @@ +#![cfg(not(windows))] +use near_contract_tools::DefaultStorageKey; +use near_sdk::serde_json::json; +use workspaces::{Account, Contract}; + +const WASM: &[u8] = + include_bytes!("../../target/wasm32-unknown-unknown/release/payroll_example.wasm"); + +struct Setup { + pub contract: Contract, + pub accounts: Vec, +} + +/// Setup for individual tests +async fn setup() -> Setup { + let worker = workspaces::sandbox().await.unwrap(); + + // Initialize user accounts + let mut accounts = Vec::new(); + for _ in 0..4 { + accounts.push(worker.dev_create_account().await.unwrap()); + } + let contract = worker.dev_deploy(&WASM.to_vec()).await.unwrap(); + + let contract_id = contract.id(); + let owner = &accounts[0]; + let manager = &accounts[1]; + let employee_1 = &accounts[2]; + let employee_2 = &accounts[3]; + + // Initialize contract + contract + .call("new") + .args_json(json!({ + "owner": owner.id() + })) + .transact() + .await + .unwrap() + .unwrap(); + + // setup roles + let result = owner + .call(contract_id, "add_manager") + .args_json(json!({ + "account_id": manager.id() + })) + .transact() + .await + .unwrap() + .unwrap(); + + let result = manager + .call(contract_id, "add_employee") + .args_json(json!({ + "account_id": employee_1.id(), + })) + .transact() + .await + .unwrap() + .unwrap(); + + let result = manager + .call(contract_id, "add_employee") + .args_json(json!({ + "account_id": employee_2.id(), + })) + .transact() + .await + .unwrap() + .unwrap(); + + Setup { contract, accounts } +} From 42ea948a5955b0b487232305b953ea627580829f Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 4 Dec 2022 19:55:11 +0800 Subject: [PATCH 06/10] Add payroll module tests --- workspaces-tests/src/bin/payroll_example.rs | 108 ++++++++++++++++++-- 1 file changed, 100 insertions(+), 8 deletions(-) diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index fef018b..7cfce13 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -126,14 +126,6 @@ struct Payroll { pub logged_time: UnorderedMap, } -/// Approval state for making payment -#[derive(BorshSerialize, BorshDeserialize, Debug)] -enum PayApprovalState { - EmployeeApproved, - ManagerApproved, - BothApproved, -} - /// Manager can add new employees and disburse payments /// Employees can log time /// @@ -162,6 +154,7 @@ impl Payroll { /// Manager can add new employees pub fn add_employee(&mut self, account_id: &AccountId) { self.require_role(&Role::Manager); + self.add_role(account_id, &Role::Employee); self.logged_time.insert(account_id, &0); } @@ -251,3 +244,102 @@ impl PayrollExternal for Payroll { } pub fn main() {} + +mod tests { + use near_contract_tools::rbac::Rbac; + use near_sdk::{test_utils::VMContextBuilder, testing_env, AccountId}; + + use crate::{Payroll, Role}; + + #[test] + fn test_owner() { + let owner: AccountId = "account_owner".parse().unwrap(); + let _contract = Payroll::new(&owner); + assert!(Payroll::has_role(&owner, &Role::Manager)); + } + + #[test] + fn test_add_manager() { + let owner: AccountId = "account_owner".parse().unwrap(); + let manager: AccountId = "account_manager".parse().unwrap(); + + let mut contract = Payroll::new(&owner); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(owner.clone()) + .build()); + + contract.add_manager(&manager); + + assert!(Payroll::has_role(&owner, &Role::Manager)); + assert!(Payroll::has_role(&manager, &Role::Manager)); + } + + #[test] + fn test_add_employees() { + let owner: AccountId = "account_owner".parse().unwrap(); + let manager: AccountId = "account_manager".parse().unwrap(); + let emp1: AccountId = "account_emp1".parse().unwrap(); + let emp2: AccountId = "account_emp2".parse().unwrap(); + + let mut contract = Payroll::new(&owner); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(owner.clone()) + .build()); + + contract.add_manager(&manager); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(manager.clone()) + .build()); + + contract.add_employee(&emp1); + contract.add_employee(&emp2); + + assert!(Payroll::has_role(&emp1, &Role::Employee)); + assert!(Payroll::has_role(&emp2, &Role::Employee)); + } + + #[test] + fn test_employee_log_time() { + let owner: AccountId = "account_owner".parse().unwrap(); + let emp1: AccountId = "account_emp1".parse().unwrap(); + let emp2: AccountId = "account_emp2".parse().unwrap(); + + let mut contract = Payroll::new(&owner); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(owner.clone()) + .build()); + + contract.add_employee(&emp1); + contract.add_employee(&emp2); + + assert!(Payroll::has_role(&emp1, &Role::Employee)); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(emp1.clone()) + .build()); + + contract.log_time(10); + assert_eq!(contract.logged_time.get(&emp1), Some(10)); + + // time is incremented correctly + contract.log_time(10); + assert_eq!(contract.logged_time.get(&emp1), Some(20)); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(emp2.clone()) + .build()); + + // time is incremented correctly for the user who made the call + contract.log_time(9); + assert_eq!(contract.logged_time.get(&emp1), Some(20)); + assert_eq!(contract.logged_time.get(&emp2), Some(9)); + + // logged_time will panic if it is called by a user with manager role + // this cannot be tested because panic unwind will need non-mutable + // reference to contract + } +} From 63b13223203706044692f801e4c5f5bae3c5474c Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 4 Dec 2022 23:05:54 +0800 Subject: [PATCH 07/10] Add more tests --- workspaces-tests/src/bin/payroll_example.rs | 34 +++++++++++++++++-- workspaces-tests/tests/payroll.rs | 37 ++++++++++++++++++--- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index 7cfce13..ffdca91 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -56,7 +56,7 @@ impl ApprovalConfiguration for Payroll { fn is_removable( &self, - action_request: &approval::ActionRequest, + _action_request: &approval::ActionRequest, ) -> Result<(), Self::RemovalError> { Ok(()) } @@ -198,10 +198,29 @@ impl Payroll { self.logged_time .insert(&employee_id, &(current_hours + hours)); } + + /// Employee can check the time they've logged + pub fn get_logged_time(&self) -> u8 { + self.require_role(&Role::Employee); + let employee_id = env::predecessor_account_id(); + + self.logged_time.get(&employee_id).unwrap_or_else(|| { + env::panic_str(format!("No record exists for account: {}", employee_id).as_str()) + }) + } + + pub fn is_employee(&self) -> bool { + self.require_role(&Role::Employee); + true + } } #[ext_contract(ext_payroll)] /// External methods for payroll +/// +/// TODO: what does externally accessible functions mean +/// if in tests different accounts can call the internal +/// functions as in workspace_tests/tests/payroll.rs pub trait PayrollExternal { /// Manager can add new managers fn payroll_add_manager(&mut self, account_id: &AccountId); @@ -245,6 +264,7 @@ impl PayrollExternal for Payroll { pub fn main() {} +#[cfg(test)] mod tests { use near_contract_tools::rbac::Rbac; use near_sdk::{test_utils::VMContextBuilder, testing_env, AccountId}; @@ -317,11 +337,11 @@ mod tests { contract.add_employee(&emp2); assert!(Payroll::has_role(&emp1, &Role::Employee)); + assert_eq!(contract.logged_time.get(&emp1), Some(0)); testing_env!(VMContextBuilder::new() .predecessor_account_id(emp1.clone()) .build()); - contract.log_time(10); assert_eq!(contract.logged_time.get(&emp1), Some(10)); @@ -338,6 +358,16 @@ mod tests { assert_eq!(contract.logged_time.get(&emp1), Some(20)); assert_eq!(contract.logged_time.get(&emp2), Some(9)); + testing_env!(VMContextBuilder::new() + .predecessor_account_id(emp1.clone()) + .build()); + assert_eq!(contract.get_logged_time(), 20); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(emp2.clone()) + .build()); + assert_eq!(contract.get_logged_time(), 9); + // logged_time will panic if it is called by a user with manager role // this cannot be tested because panic unwind will need non-mutable // reference to contract diff --git a/workspaces-tests/tests/payroll.rs b/workspaces-tests/tests/payroll.rs index ecada99..bea4220 100644 --- a/workspaces-tests/tests/payroll.rs +++ b/workspaces-tests/tests/payroll.rs @@ -25,8 +25,8 @@ async fn setup() -> Setup { let contract_id = contract.id(); let owner = &accounts[0]; let manager = &accounts[1]; - let employee_1 = &accounts[2]; - let employee_2 = &accounts[3]; + let emp1 = &accounts[2]; + let emp2 = &accounts[3]; // Initialize contract contract @@ -53,7 +53,7 @@ async fn setup() -> Setup { let result = manager .call(contract_id, "add_employee") .args_json(json!({ - "account_id": employee_1.id(), + "account_id": emp1.id(), })) .transact() .await @@ -63,7 +63,7 @@ async fn setup() -> Setup { let result = manager .call(contract_id, "add_employee") .args_json(json!({ - "account_id": employee_2.id(), + "account_id": emp2.id(), })) .transact() .await @@ -72,3 +72,32 @@ async fn setup() -> Setup { Setup { contract, accounts } } + +#[tokio::test] +async fn successful_request() { + let Setup { contract, accounts } = setup().await; + + let contract_id = contract.id(); + let owner = &accounts[0]; + let manager = &accounts[1]; + let emp1 = &accounts[2]; + let emp2 = &accounts[3]; + + let result = emp1 + .call(contract_id, "is_employee") + .transact() + .await + .unwrap() + .unwrap(); + + dbg!(&result.borsh::(), true); + + let result = emp1 + .call(contract_id, "get_logged_time") + .transact() + .await + .unwrap() + .unwrap(); + + dbg!(&result.borsh::(), 0); +} From a4fbcf75755d563f238a2431427429019c56eecd Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sat, 17 Dec 2022 23:51:27 +0800 Subject: [PATCH 08/10] Change approval configuration and add more tests --- workspaces-tests/src/bin/payroll_example.rs | 8 +- workspaces-tests/tests/payroll.rs | 135 ++++++++++++++++++-- 2 files changed, 131 insertions(+), 12 deletions(-) diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index ffdca91..ce2ec8f 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -37,7 +37,10 @@ pub enum PayrollApproval { BothApproved, } -impl ApprovalConfiguration for Payroll { +#[derive(BorshSerialize, BorshDeserialize, Serialize)] +struct PayApprovalConfiguration; + +impl ApprovalConfiguration for PayApprovalConfiguration { type ApprovalError = String; type RemovalError = (); type AuthorizationError = String; @@ -110,7 +113,7 @@ impl ApprovalConfiguration for Payroll { } } -impl ApprovalManager for Payroll {} +impl ApprovalManager for Payroll {} #[derive(BorshSerialize, BorshDeserialize, BorshStorageKey)] enum Role { @@ -141,6 +144,7 @@ impl Payroll { hourly_fee: 1000, logged_time: UnorderedMap::new(PayrollKey::LOG), }; + >::init(PayApprovalConfiguration {}); contract.add_role(owner, &Role::Manager); contract } diff --git a/workspaces-tests/tests/payroll.rs b/workspaces-tests/tests/payroll.rs index bea4220..e721393 100644 --- a/workspaces-tests/tests/payroll.rs +++ b/workspaces-tests/tests/payroll.rs @@ -1,7 +1,7 @@ #![cfg(not(windows))] -use near_contract_tools::DefaultStorageKey; use near_sdk::serde_json::json; -use workspaces::{Account, Contract}; +use std::str; +use workspaces::{result::ExecutionFailure, Account, Contract}; const WASM: &[u8] = include_bytes!("../../target/wasm32-unknown-unknown/release/payroll_example.wasm"); @@ -40,7 +40,7 @@ async fn setup() -> Setup { .unwrap(); // setup roles - let result = owner + owner .call(contract_id, "add_manager") .args_json(json!({ "account_id": manager.id() @@ -50,7 +50,7 @@ async fn setup() -> Setup { .unwrap() .unwrap(); - let result = manager + manager .call(contract_id, "add_employee") .args_json(json!({ "account_id": emp1.id(), @@ -60,7 +60,7 @@ async fn setup() -> Setup { .unwrap() .unwrap(); - let result = manager + manager .call(contract_id, "add_employee") .args_json(json!({ "account_id": emp2.id(), @@ -73,15 +73,14 @@ async fn setup() -> Setup { Setup { contract, accounts } } +/// Test that setup and employee apis for checking status and logged hours #[tokio::test] -async fn successful_request() { +async fn test_setup() { let Setup { contract, accounts } = setup().await; let contract_id = contract.id(); - let owner = &accounts[0]; let manager = &accounts[1]; let emp1 = &accounts[2]; - let emp2 = &accounts[3]; let result = emp1 .call(contract_id, "is_employee") @@ -90,7 +89,64 @@ async fn successful_request() { .unwrap() .unwrap(); - dbg!(&result.borsh::(), true); + assert_eq!( + str::from_utf8(&result.raw_bytes().unwrap()).unwrap(), + "true" + ); + + let result = emp1 + .call(contract_id, "get_logged_time") + .transact() + .await + .unwrap() + .unwrap(); + + assert_eq!(str::from_utf8(&result.raw_bytes().unwrap()).unwrap(), "0"); + + // Only employee can log time + assert!(manager + .call(contract_id, "get_logged_time") + .transact() + .await + .unwrap() + .is_failure()) +} + +/// Test log time correctly logs time for the callee +#[tokio::test] +async fn test_log_time() { + let Setup { contract, accounts } = setup().await; + + let contract_id = contract.id(); + let emp1 = &accounts[2]; + let emp2 = &accounts[3]; + + emp1.call(contract_id, "log_time") + .args_json(json!({ + "hours": 10, + })) + .transact() + .await + .unwrap() + .unwrap(); + + emp2.call(contract_id, "log_time") + .args_json(json!({ + "hours": 9, + })) + .transact() + .await + .unwrap() + .unwrap(); + + emp1.call(contract_id, "log_time") + .args_json(json!({ + "hours": 10, + })) + .transact() + .await + .unwrap() + .unwrap(); let result = emp1 .call(contract_id, "get_logged_time") @@ -99,5 +155,64 @@ async fn successful_request() { .unwrap() .unwrap(); - dbg!(&result.borsh::(), 0); + assert_eq!(str::from_utf8(&result.raw_bytes().unwrap()).unwrap(), "20"); + + let result = emp2 + .call(contract_id, "get_logged_time") + .transact() + .await + .unwrap() + .unwrap(); + + assert_eq!(str::from_utf8(&result.raw_bytes().unwrap()).unwrap(), "9"); +} + +#[tokio::test] +async fn test_disburse_payment() { + let Setup { contract, accounts } = setup().await; + + let contract_id = contract.id(); + let manager = &accounts[1]; + let emp1 = &accounts[2]; + + emp1.call(contract_id, "log_time") + .args_json(json!({ + "hours": 10, + })) + .transact() + .await + .unwrap() + .unwrap(); + + let request_id = emp1 + .call(contract_id, "request_pay") + .transact() + .await + .unwrap() + .unwrap(); + + let request_id = str::from_utf8(&request_id.raw_bytes().unwrap()) + .unwrap() + .parse::() + .expect("request_pay response should be an integer"); + + manager + .call(contract_id, "approve_pay") + .args_json(json!({ + "request_id": request_id, + })) + .transact() + .await + .unwrap() + .unwrap(); + + let promise = emp1 + .call(contract_id, "log_time") + .args_json(json!({ + "hours": 10, + })) + .transact() + .await + .unwrap() + .unwrap(); } From 3e7745732ec865a08a8d53453c65929a98d3710b Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 18 Dec 2022 00:07:31 +0800 Subject: [PATCH 09/10] Fix test --- workspaces-tests/tests/payroll.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/workspaces-tests/tests/payroll.rs b/workspaces-tests/tests/payroll.rs index e721393..48bad72 100644 --- a/workspaces-tests/tests/payroll.rs +++ b/workspaces-tests/tests/payroll.rs @@ -1,7 +1,7 @@ #![cfg(not(windows))] use near_sdk::serde_json::json; use std::str; -use workspaces::{result::ExecutionFailure, Account, Contract}; +use workspaces::{Account, Contract}; const WASM: &[u8] = include_bytes!("../../target/wasm32-unknown-unknown/release/payroll_example.wasm"); @@ -206,13 +206,15 @@ async fn test_disburse_payment() { .unwrap() .unwrap(); - let promise = emp1 - .call(contract_id, "log_time") - .args_json(json!({ - "hours": 10, - })) + let balance_before = emp1.view_account().await.unwrap().balance; + + emp1.call(contract_id, "get_pay") .transact() .await .unwrap() .unwrap(); + + let balance_after = emp1.view_account().await.unwrap().balance; + + assert_eq!(balance_after - balance_before, 10000); } From 8dec7c8df4a9bc59ffa3d539b66a956f951d04c5 Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 18 Dec 2022 11:57:45 +0800 Subject: [PATCH 10/10] Clean up implementation --- workspaces-tests/src/bin/payroll_example.rs | 109 ++++++-------------- 1 file changed, 34 insertions(+), 75 deletions(-) diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs index ce2ec8f..3cfdce0 100644 --- a/workspaces-tests/src/bin/payroll_example.rs +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -1,16 +1,20 @@ //! Payroll system manages employee and their pay -use near_contract_tools::{ - approval::{self, ApprovalConfiguration, ApprovalManager}, - rbac::Rbac, - Rbac, -}; use near_sdk::{ borsh::{self, BorshDeserialize, BorshSerialize}, collections::UnorderedMap, - env, ext_contract, near_bindgen, + env, near_bindgen, serde::Serialize, AccountId, BorshStorageKey, PanicOnDefault, Promise, }; +use near_sdk_contract_tools::{ + approval::{self, ApprovalConfiguration, ApprovalManager}, + // TODO: It is a bit confusing the for a beginner to import both the trait + // and the macro of the same name and similar path. It can be clarified + // either through documentation, slightly differently named module paths, + // by slightly different proc macro names. + rbac::Rbac, + Rbac, +}; #[derive(BorshStorageKey, BorshSerialize)] enum PayrollKey { @@ -31,6 +35,7 @@ impl approval::Action for PayrollAction { /// Both manager and employee need to approve payment request #[derive(BorshSerialize, BorshDeserialize, Serialize)] +#[serde(crate = "near_sdk::serde")] pub enum PayrollApproval { EmployeeApproved, ManagerApproved, @@ -38,6 +43,7 @@ pub enum PayrollApproval { } #[derive(BorshSerialize, BorshDeserialize, Serialize)] +#[serde(crate = "near_sdk::serde")] struct PayApprovalConfiguration; impl ApprovalConfiguration for PayApprovalConfiguration { @@ -139,7 +145,7 @@ struct Payroll { #[near_bindgen] impl Payroll { #[init] - pub fn new(owner: &AccountId) -> Self { + pub fn new(owner: AccountId) -> Self { let mut contract = Payroll { hourly_fee: 1000, logged_time: UnorderedMap::new(PayrollKey::LOG), @@ -150,16 +156,16 @@ impl Payroll { } /// Manager can add new managers - pub fn add_manager(&mut self, account_id: &AccountId) { - self.require_role(&Role::Manager); + pub fn add_manager(&mut self, account_id: AccountId) { + Payroll::require_role(&Role::Manager); self.add_role(account_id, &Role::Manager); } /// Manager can add new employees - pub fn add_employee(&mut self, account_id: &AccountId) { - self.require_role(&Role::Manager); - self.add_role(account_id, &Role::Employee); - self.logged_time.insert(account_id, &0); + pub fn add_employee(&mut self, account_id: AccountId) { + Payroll::require_role(&Role::Manager); + self.add_role(account_id.clone(), &Role::Employee); + self.logged_time.insert(&account_id, &0); } pub fn approve_pay(&mut self, request_id: u32) { @@ -172,7 +178,7 @@ impl Payroll { /// Employee can request pay pub fn request_pay(&mut self) -> u32 { - self.require_role(&Role::Employee); + Payroll::require_role(&Role::Employee); let employee_id = env::predecessor_account_id(); let logged_time = self.logged_time.get(&employee_id).unwrap_or_else(|| { env::panic_str(format!("No record exists for account: {}", employee_id).as_str()) @@ -192,7 +198,7 @@ impl Payroll { /// Employee can log time pub fn log_time(&mut self, hours: u8) { - self.require_role(&Role::Employee); + Payroll::require_role(&Role::Employee); let employee_id = env::predecessor_account_id(); let current_hours = self.logged_time.get(&employee_id).unwrap_or_else(|| { env::panic_str(format!("No record exists for account: {}", employee_id).as_str()) @@ -205,7 +211,7 @@ impl Payroll { /// Employee can check the time they've logged pub fn get_logged_time(&self) -> u8 { - self.require_role(&Role::Employee); + Payroll::require_role(&Role::Employee); let employee_id = env::predecessor_account_id(); self.logged_time.get(&employee_id).unwrap_or_else(|| { @@ -214,71 +220,24 @@ impl Payroll { } pub fn is_employee(&self) -> bool { - self.require_role(&Role::Employee); + Payroll::require_role(&Role::Employee); true } } -#[ext_contract(ext_payroll)] -/// External methods for payroll -/// -/// TODO: what does externally accessible functions mean -/// if in tests different accounts can call the internal -/// functions as in workspace_tests/tests/payroll.rs -pub trait PayrollExternal { - /// Manager can add new managers - fn payroll_add_manager(&mut self, account_id: &AccountId); - /// Manager can add new employees - fn payroll_add_employee(&mut self, account_id: AccountId); - /// Employee can log time - fn payroll_log_time(&mut self, hours: u8); - /// Employee can request for pay - fn payroll_request_pay(&mut self) -> u32; - /// Pay can be approved by manager and the employee who made the request - fn payroll_approve_pay(&mut self, request_id: u32); - /// If request is approved get_pay will return a promise to transfer funds - fn payroll_get_pay(&mut self, request_id: u32) -> Promise; -} - -impl PayrollExternal for Payroll { - fn payroll_add_manager(&mut self, account_id: &AccountId) { - self.add_manager(account_id); - } - - fn payroll_add_employee(&mut self, account_id: AccountId) { - self.add_employee(&account_id); - } - - fn payroll_log_time(&mut self, hours: u8) { - self.log_time(hours); - } - - fn payroll_request_pay(&mut self) -> u32 { - self.request_pay() - } - - fn payroll_approve_pay(&mut self, request_id: u32) { - self.approve_pay(request_id); - } - - fn payroll_get_pay(&mut self, request_id: u32) -> Promise { - self.get_pay(request_id) - } -} - pub fn main() {} #[cfg(test)] mod tests { - use near_contract_tools::rbac::Rbac; use near_sdk::{test_utils::VMContextBuilder, testing_env, AccountId}; + use near_sdk_contract_tools::rbac::Rbac; use crate::{Payroll, Role}; #[test] fn test_owner() { let owner: AccountId = "account_owner".parse().unwrap(); - let _contract = Payroll::new(&owner); + let _contract = Payroll::new(owner.clone()); assert!(Payroll::has_role(&owner, &Role::Manager)); } @@ -287,13 +246,13 @@ mod tests { let owner: AccountId = "account_owner".parse().unwrap(); let manager: AccountId = "account_manager".parse().unwrap(); - let mut contract = Payroll::new(&owner); + let mut contract = Payroll::new(owner.clone()); testing_env!(VMContextBuilder::new() .predecessor_account_id(owner.clone()) .build()); - contract.add_manager(&manager); + contract.add_manager(manager.clone()); assert!(Payroll::has_role(&owner, &Role::Manager)); assert!(Payroll::has_role(&manager, &Role::Manager)); @@ -306,20 +265,20 @@ mod tests { let emp1: AccountId = "account_emp1".parse().unwrap(); let emp2: AccountId = "account_emp2".parse().unwrap(); - let mut contract = Payroll::new(&owner); + let mut contract = Payroll::new(owner.clone()); testing_env!(VMContextBuilder::new() .predecessor_account_id(owner.clone()) .build()); - contract.add_manager(&manager); + contract.add_manager(manager.clone()); testing_env!(VMContextBuilder::new() .predecessor_account_id(manager.clone()) .build()); - contract.add_employee(&emp1); - contract.add_employee(&emp2); + contract.add_employee(emp1.clone()); + contract.add_employee(emp2.clone()); assert!(Payroll::has_role(&emp1, &Role::Employee)); assert!(Payroll::has_role(&emp2, &Role::Employee)); @@ -331,14 +290,14 @@ mod tests { let emp1: AccountId = "account_emp1".parse().unwrap(); let emp2: AccountId = "account_emp2".parse().unwrap(); - let mut contract = Payroll::new(&owner); + let mut contract = Payroll::new(owner.clone()); testing_env!(VMContextBuilder::new() .predecessor_account_id(owner.clone()) .build()); - contract.add_employee(&emp1); - contract.add_employee(&emp2); + contract.add_employee(emp1.clone()); + contract.add_employee(emp2.clone()); assert!(Payroll::has_role(&emp1, &Role::Employee)); assert_eq!(contract.logged_time.get(&emp1), Some(0));