diff --git a/src/approval/mod.rs b/src/approval/mod.rs index e85454e..8ad16b6 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 6d7c475..f2e0dbb 100644 --- a/workspaces-tests/Cargo.toml +++ b/workspaces-tests/Cargo.toml @@ -41,6 +41,9 @@ name = "upgrade_old_multisig" [[bin]] name = "upgrade_old_raw" +[[bin]] +name = "payroll_example" + [dependencies] near-sdk-contract-tools = {path = "../", features = ["unstable"]} near-sdk = { version = "4.1.1", default-features = false } diff --git a/workspaces-tests/src/bin/payroll_example.rs b/workspaces-tests/src/bin/payroll_example.rs new file mode 100644 index 0000000..3cfdce0 --- /dev/null +++ b/workspaces-tests/src/bin/payroll_example.rs @@ -0,0 +1,338 @@ +//! Payroll system manages employee and their pay +use near_sdk::{ + borsh::{self, BorshDeserialize, BorshSerialize}, + collections::UnorderedMap, + 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 { + LOG, +} + +#[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)] +#[serde(crate = "near_sdk::serde")] +pub enum PayrollApproval { + EmployeeApproved, + ManagerApproved, + BothApproved, +} + +#[derive(BorshSerialize, BorshDeserialize, Serialize)] +#[serde(crate = "near_sdk::serde")] +struct PayApprovalConfiguration; + +impl ApprovalConfiguration for PayApprovalConfiguration { + 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, + 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 +/// +/// 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 { + #[init] + pub fn new(owner: AccountId) -> Self { + let mut contract = Payroll { + hourly_fee: 1000, + logged_time: UnorderedMap::new(PayrollKey::LOG), + }; + >::init(PayApprovalConfiguration {}); + contract.add_role(owner, &Role::Manager); + contract + } + + /// Manager can add new managers + 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) { + 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) { + 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(&mut self) -> u32 { + 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()) + }); + + 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 + pub fn log_time(&mut self, hours: u8) { + 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()) + }); + + // Add entry for employee's account id + self.logged_time + .insert(&employee_id, &(current_hours + hours)); + } + + /// Employee can check the time they've logged + pub fn get_logged_time(&self) -> u8 { + Payroll::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 { + Payroll::require_role(&Role::Employee); + true + } +} + +pub fn main() {} + +#[cfg(test)] +mod tests { + 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.clone()); + 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.clone()); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(owner.clone()) + .build()); + + contract.add_manager(manager.clone()); + + 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.clone()); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(owner.clone()) + .build()); + + contract.add_manager(manager.clone()); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(manager.clone()) + .build()); + + contract.add_employee(emp1.clone()); + contract.add_employee(emp2.clone()); + + 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.clone()); + + testing_env!(VMContextBuilder::new() + .predecessor_account_id(owner.clone()) + .build()); + + 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)); + + 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)); + + 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 new file mode 100644 index 0000000..48bad72 --- /dev/null +++ b/workspaces-tests/tests/payroll.rs @@ -0,0 +1,220 @@ +#![cfg(not(windows))] +use near_sdk::serde_json::json; +use std::str; +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 emp1 = &accounts[2]; + let emp2 = &accounts[3]; + + // Initialize contract + contract + .call("new") + .args_json(json!({ + "owner": owner.id() + })) + .transact() + .await + .unwrap() + .unwrap(); + + // setup roles + owner + .call(contract_id, "add_manager") + .args_json(json!({ + "account_id": manager.id() + })) + .transact() + .await + .unwrap() + .unwrap(); + + manager + .call(contract_id, "add_employee") + .args_json(json!({ + "account_id": emp1.id(), + })) + .transact() + .await + .unwrap() + .unwrap(); + + manager + .call(contract_id, "add_employee") + .args_json(json!({ + "account_id": emp2.id(), + })) + .transact() + .await + .unwrap() + .unwrap(); + + Setup { contract, accounts } +} + +/// Test that setup and employee apis for checking status and logged hours +#[tokio::test] +async fn test_setup() { + let Setup { contract, accounts } = setup().await; + + let contract_id = contract.id(); + let manager = &accounts[1]; + let emp1 = &accounts[2]; + + let result = emp1 + .call(contract_id, "is_employee") + .transact() + .await + .unwrap() + .unwrap(); + + 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") + .transact() + .await + .unwrap() + .unwrap(); + + 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 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); +}