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: extract ownership management into sharable interface #97

Merged
merged 9 commits into from
Dec 6, 2024
Merged
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
5 changes: 4 additions & 1 deletion contracts/axelar-gas-service/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,13 @@ impl UpgradableInterface for AxelarGasService {

#[contractimpl]
impl OwnableInterface for AxelarGasService {
// boilerplate necessary for the contractimpl macro to include function in the generated client
fn owner(env: &Env) -> Address {
interfaces::owner(env)
}

fn transfer_ownership(env: &Env, new_owner: Address) {
interfaces::transfer_ownership::<Self>(env, new_owner);
}
}

#[contractimpl]
Expand Down
1 change: 0 additions & 1 deletion contracts/axelar-gas-service/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use axelar_gas_service::error::ContractError;
use axelar_soroban_std::{
assert_contract_err, assert_invoke_auth_err, assert_last_emitted_event, types::Token,
};
use soroban_sdk::testutils::{MockAuth, MockAuthInvoke};
use soroban_sdk::Bytes;
use soroban_sdk::{
bytes,
Expand Down
14 changes: 4 additions & 10 deletions contracts/axelar-gateway/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,13 @@ impl UpgradableInterface for AxelarGateway {

#[contractimpl]
impl OwnableInterface for AxelarGateway {
// boilerplate necessary for the contractimpl macro to include function in the generated client
fn owner(env: &Env) -> Address {
interfaces::owner(env)
}

fn transfer_ownership(env: &Env, new_owner: Address) {
interfaces::transfer_ownership::<Self>(env, new_owner);
}
}

#[contractimpl]
Expand Down Expand Up @@ -251,15 +254,6 @@ impl AxelarGatewayInterface for AxelarGateway {
auth::epoch(env)
}

fn transfer_ownership(env: Env, new_owner: Address) {
let owner: Address = Self::owner(&env);
owner.require_auth();

interfaces::set_owner(&env, &new_owner);

event::transfer_ownership(&env, owner, new_owner);
}

fn epoch_by_signers_hash(env: &Env, signers_hash: BytesN<32>) -> Result<u64, ContractError> {
auth::epoch_by_signers_hash(env, signers_hash)
}
Expand Down
9 changes: 0 additions & 9 deletions contracts/axelar-gateway/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,3 @@ pub fn transfer_operatorship(env: &Env, previous_operator: Address, new_operator
);
env.events().publish(topics, ());
}

pub fn transfer_ownership(env: &Env, previous_owner: Address, new_owner: Address) {
let topics = (
Symbol::new(env, "ownership_transferred"),
previous_owner,
new_owner,
);
env.events().publish(topics, ());
}
3 changes: 0 additions & 3 deletions contracts/axelar-gateway/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ pub trait AxelarGatewayInterface: AxelarGatewayMessagingInterface + UpgradableIn
/// Returns the epoch of the gateway.
fn epoch(env: &Env) -> u64;

/// Transfers ownership of the gateway to a new address.
fn transfer_ownership(env: Env, new_owner: Address);

/// Returns the epoch by signers hash.
fn epoch_by_signers_hash(env: &Env, signers_hash: BytesN<32>) -> Result<u64, ContractError>;

Expand Down
2 changes: 1 addition & 1 deletion contracts/axelar-gateway/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod interface;
#[cfg(all(target_family = "wasm", feature = "testutils"))]
compile_error!("'testutils' feature is not supported on 'wasm' target");

#[cfg(feature = "testutils")]
#[cfg(any(test, feature = "testutils"))]
pub mod testutils;

cfg_if::cfg_if! {
Expand Down
25 changes: 2 additions & 23 deletions contracts/axelar-gateway/tests/test.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use axelar_gateway::error::ContractError;
#[cfg(any(test, feature = "testutils"))]
use axelar_gateway::testutils::{
generate_proof, generate_signers_set, generate_test_message, get_approve_hash, randint,
setup_gateway, TestSignerSet,
Expand All @@ -12,7 +13,7 @@ use axelar_soroban_std::{
use soroban_sdk::Symbol;
use soroban_sdk::{
bytes,
testutils::{Address as _, Events, MockAuth, MockAuthInvoke},
testutils::{Address as _, Events},
vec, Address, BytesN, Env, String,
};

Expand Down Expand Up @@ -366,28 +367,6 @@ fn transfer_operatorship_unauthorized() {
);
}

#[test]
fn transfer_ownership() {
let (env, _signers, client) = setup_env(1, randint(1, 10));

let owner = client.owner();
let new_owner = Address::generate(&env);

assert_invoke_auth_ok!(owner, client.try_transfer_ownership(&new_owner));
assert_last_emitted_event(
&env,
&client.address,
(
Symbol::new(&env, "ownership_transferred"),
owner,
new_owner.clone(),
),
(),
);

assert_eq!(client.owner(), new_owner);
}

#[test]
fn transfer_ownership_unauthorized() {
let (env, _, client) = setup_env(1, randint(1, 10));
Expand Down
17 changes: 4 additions & 13 deletions contracts/axelar-operators/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,6 @@ impl AxelarOperators {
interfaces::set_owner(&env, &owner);
}

pub fn transfer_ownership(env: Env, new_owner: Address) -> Result<(), ContractError> {
let owner: Address = Self::owner(&env);

owner.require_auth();

interfaces::set_owner(&env, &new_owner);

event::transfer_ownership(&env, owner, new_owner);

Ok(())
}

/// Return true if the account is an operator.
pub fn is_operator(env: Env, account: Address) -> bool {
let key = DataKey::Operators(account);
Expand Down Expand Up @@ -123,8 +111,11 @@ impl UpgradableInterface for AxelarOperators {

#[contractimpl]
impl OwnableInterface for AxelarOperators {
// boilerplate necessary for the contractimpl macro to include function in the generated client
fn owner(env: &Env) -> Address {
interfaces::owner(env)
}

fn transfer_ownership(env: &Env, new_owner: Address) {
interfaces::transfer_ownership::<Self>(env, new_owner);
}
}
9 changes: 0 additions & 9 deletions contracts/axelar-operators/src/event.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
use soroban_sdk::{Address, Env, Symbol};

pub fn transfer_ownership(env: &Env, previous_owner: Address, new_owner: Address) {
let topics = (
Symbol::new(env, "ownership_transferred"),
previous_owner,
new_owner,
);
env.events().publish(topics, ());
}

pub fn add_operator(env: &Env, operator: Address) {
let topics = (Symbol::new(env, "operator_added"), operator);
env.events().publish(topics, ());
Expand Down
37 changes: 1 addition & 36 deletions contracts/axelar-operators/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use axelar_soroban_std::{

use axelar_operators::contract::{AxelarOperators, AxelarOperatorsClient};
use soroban_sdk::{
contract, contractimpl, symbol_short,
testutils::{Address as _, MockAuth, MockAuthInvoke},
Address, Env, Symbol, Val, Vec,
contract, contractimpl, symbol_short, testutils::Address as _, Address, Env, Symbol, Val, Vec,
};

#[contract]
Expand Down Expand Up @@ -51,39 +49,6 @@ fn register_operators() {
assert_eq!(client.owner(), user);
}

#[test]
fn transfer_owner() {
let (env, client, _) = setup_env();

let initial_owner = client.owner();
let new_owner = Address::generate(&env);

// transfer ownership to the new owner
client.transfer_ownership(&new_owner);

assert_invocation(
&env,
&initial_owner,
&client.address,
"transfer_ownership",
(new_owner.clone(),),
);

assert_last_emitted_event(
&env,
&client.address,
(
Symbol::new(&env, "ownership_transferred"),
initial_owner,
new_owner.clone(),
),
(),
);

let retrieved_owner = client.owner();
assert_eq!(retrieved_owner, new_owner);
}

#[test]
fn add_operator() {
let (env, client, _) = setup_env();
Expand Down
14 changes: 4 additions & 10 deletions contracts/interchain-token-service/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,6 @@ impl InterchainTokenService {

#[contractimpl]
impl InterchainTokenServiceInterface for InterchainTokenService {
fn transfer_ownership(env: &Env, new_owner: Address) {
let owner = Self::owner(env);
owner.require_auth();

interfaces::set_owner(env, &new_owner);

event::transfer_ownership(env, owner, new_owner);
}

fn trusted_address(env: &Env, chain: String) -> Option<String> {
env.storage()
.persistent()
Expand Down Expand Up @@ -303,8 +294,11 @@ impl UpgradableInterface for InterchainTokenService {

#[contractimpl]
impl OwnableInterface for InterchainTokenService {
// boilerplate necessary for the contractimpl macro to include function in the generated client
fn owner(env: &Env) -> Address {
interfaces::owner(env)
}

fn transfer_ownership(env: &Env, new_owner: Address) {
interfaces::transfer_ownership::<Self>(env, new_owner);
}
}
11 changes: 1 addition & 10 deletions contracts/interchain-token-service/src/event.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use soroban_sdk::{Address, Bytes, Env, String, Symbol};
use soroban_sdk::{Bytes, Env, String, Symbol};

pub fn set_trusted_address(env: &Env, chain: String, trusted_address: String) {
let topics = (
Expand All @@ -18,15 +18,6 @@ pub fn remove_trusted_address(env: &Env, chain: String, trusted_address: String)
env.events().publish(topics, ());
}

pub fn transfer_ownership(env: &Env, previous_owner: Address, new_owner: Address) {
let topics = (
Symbol::new(env, "ownership_transferred"),
previous_owner,
new_owner,
);
env.events().publish(topics, ());
}

pub fn executed(
env: &Env,
source_chain: String,
Expand Down
2 changes: 0 additions & 2 deletions contracts/interchain-token-service/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use crate::error::ContractError;

#[contractclient(name = "InterchainTokenServiceClient")]
pub trait InterchainTokenServiceInterface: AxelarExecutableInterface {
fn transfer_ownership(env: &Env, new_owner: Address);

fn trusted_address(env: &Env, chain: String) -> Option<String>;

fn set_trusted_address(env: &Env, chain: String, address: String) -> Result<(), ContractError>;
Expand Down
25 changes: 0 additions & 25 deletions contracts/interchain-token-service/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use interchain_token_service::contract::{InterchainTokenService, InterchainToken
use interchain_token_service::error::ContractError;

use axelar_soroban_std::{assert_contract_err, assert_invoke_auth_err, assert_last_emitted_event};
use soroban_sdk::testutils::{MockAuth, MockAuthInvoke};

use soroban_sdk::BytesN;
use soroban_sdk::{testutils::Address as _, Address, Env, String, Symbol};
Expand Down Expand Up @@ -165,30 +164,6 @@ fn remove_trusted_address_fails_if_address_not_set() {
);
}

#[test]
fn transfer_ownership() {
let (env, client) = setup_env();
env.mock_all_auths();

let prev_owner = client.owner();
let new_owner = Address::generate(&env);

client.transfer_ownership(&new_owner);

assert_last_emitted_event(
&env,
&client.address,
(
Symbol::new(&env, "ownership_transferred"),
prev_owner,
new_owner.clone(),
),
(),
);

assert_eq!(client.owner(), new_owner);
}

#[test]
fn interchain_token_deploy_salt() {
let (env, client) = setup_env();
Expand Down
24 changes: 7 additions & 17 deletions contracts/interchain-token/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use axelar_soroban_std::{ensure, interfaces};
use soroban_sdk::token::TokenInterface;

use soroban_sdk::{assert_with_error, contract, contractimpl, token, Address, BytesN, Env, String};
use soroban_token_sdk::event::Events as TokenEvents;

#[contract]
pub struct InterchainToken;
Expand Down Expand Up @@ -106,22 +107,6 @@ impl InterchainTokenInterface for InterchainToken {

event::remove_minter(env, minter);
}

fn transfer_ownership(env: Env, new_owner: Address) -> Result<(), ContractError> {
let owner: Address = Self::owner(&env);

owner.require_auth();

interfaces::set_owner(&env, &new_owner);

TokenUtils::new(&env)
.events()
.set_admin(owner.clone(), new_owner.clone());

event::transfer_ownership(&env, owner, new_owner);

Ok(())
}
}

#[contractimpl]
Expand Down Expand Up @@ -382,8 +367,13 @@ impl UpgradableInterface for InterchainToken {

#[contractimpl]
impl OwnableInterface for InterchainToken {
// boilerplate necessary for the contractimpl macro to include function in the generated client
fn owner(env: &Env) -> Address {
interfaces::owner(env)
}

fn transfer_ownership(env: &Env, new_owner: Address) {
interfaces::transfer_ownership::<Self>(env, new_owner.clone());
// adhere to reference implementation for tokens and emit predefined soroban event
TokenEvents::new(env).set_admin(Self::owner(env), new_owner);
}
}
9 changes: 0 additions & 9 deletions contracts/interchain-token/src/event.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
use soroban_sdk::{Address, Env, Symbol};

pub fn transfer_ownership(env: &Env, previous_owner: Address, new_owner: Address) {
let topics = (
Symbol::new(env, "ownership_transferred"),
previous_owner,
new_owner,
);
env.events().publish(topics, ());
}

pub fn add_minter(env: &Env, minter: Address) {
let topics = (Symbol::new(env, "minter_added"), minter);
env.events().publish(topics, ());
Expand Down
2 changes: 0 additions & 2 deletions contracts/interchain-token/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,4 @@ pub trait InterchainTokenInterface: token::Interface {
fn mint(env: Env, minter: Address, to: Address, amount: i128) -> Result<(), ContractError>;
fn add_minter(env: &Env, minter: Address);
fn remove_minter(env: &Env, minter: Address);

fn transfer_ownership(env: Env, new_owner: Address) -> Result<(), ContractError>;
}
Loading
Loading