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

fix: Don't use env::signer_account_id #524

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
9 changes: 4 additions & 5 deletions contracts/near/admin-controlled/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
pub mod macros;
pub use macros::*;

use near_sdk::env;

pub type Mask = u128;

pub trait AdminControlled {
fn is_owner(&self) -> bool {
env::current_account_id() == env::signer_account_id()
}

fn assert_owner(&self) {
assert!(self.is_owner());
env::current_account_id() == env::predecessor_account_id()
}

/// Return the current mask representing all paused events.
Expand Down
21 changes: 21 additions & 0 deletions contracts/near/admin-controlled/src/macros.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#[macro_export]
macro_rules! impl_admin_controlled {
($contract: ident, $paused: ident) => {
use admin_controlled::{AdminControlled as AdminControlledInner, Mask as MaskInner};
use near_sdk as near_sdk_inner;
Copy link
Contributor

Choose a reason for hiding this comment

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

Using use in macros is not a very good idea, because it silently imports names into the surrounding namespace. Also, it assumes that near_sdk and admin_controlled crates are present in the list of dependencies of the current crate under their original names.

A more robust solution would be to not use use and to use fully qualifying names starting with $crate (because $crate is the only top-level name such that you know for sure what it refers to).


#[near_bindgen]
impl AdminControlledInner for $contract {
#[result_serializer(borsh)]
fn get_paused(&self) -> MaskInner {
self.$paused
}

#[result_serializer(borsh)]
fn set_paused(&mut self, #[serializer(borsh)] paused: MaskInner) {
near_sdk_inner::assert_self();
self.$paused = paused;
}
}
};
}
18 changes: 3 additions & 15 deletions contracts/near/eth-client/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use admin_controlled::{AdminControlled, Mask};
use admin_controlled::Mask;
use borsh::{BorshDeserialize, BorshSerialize};
use eth_types::*;
use near_sdk::collections::UnorderedMap;
Expand Down Expand Up @@ -111,20 +111,6 @@ impl Default for EthClient {
}
}

#[near_bindgen]
impl AdminControlled for EthClient {
#[result_serializer(borsh)]
fn get_paused(&self) -> Mask {
self.paused
}

#[result_serializer(borsh)]
fn set_paused(&mut self, #[serializer(borsh)] paused: Mask) {
self.assert_owner();
self.paused = paused;
}
}

#[near_bindgen]
impl EthClient {
#[init]
Expand Down Expand Up @@ -441,3 +427,5 @@ impl EthClient {
(H256(pair.0), H256(pair.1))
}
}

admin_controlled::impl_admin_controlled!(EthClient, paused);
16 changes: 2 additions & 14 deletions contracts/near/eth-prover/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use admin_controlled::{AdminControlled, Mask};
use admin_controlled::Mask;
use borsh::{BorshDeserialize, BorshSerialize};
use eth_types::*;
use near_sdk::{env, ext_contract, near_bindgen, PromiseOrValue};
Expand Down Expand Up @@ -320,16 +320,4 @@ impl EthProver {
}
}

#[near_bindgen]
impl AdminControlled for EthProver {
#[result_serializer(borsh)]
fn get_paused(&self) -> Mask {
self.paused
}

#[result_serializer(borsh)]
fn set_paused(&mut self, #[serializer(borsh)] paused: Mask) {
self.assert_owner();
self.paused = paused;
}
}
admin_controlled::impl_admin_controlled!(EthProver, paused);