Skip to content

Commit

Permalink
feat: audit address comments (#28)
Browse files Browse the repository at this point in the history 
* Addressed all relevant audit comments

* added some comments

* fixed tests

* Update move/axelar/sources/gateway.move

* Update move/axelar/sources/gateway.move

* rename

---------

Co-authored-by: Milap Sheth <[email protected]>
  • Loading branch information
@Foivos @milapsheth
Foivos and milapsheth authored May 26, 2024
1 parent 9b4b12f commit a42642e
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 631 deletions.
52 changes: 36 additions & 16 deletions move/axelar/sources/gateway.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ module axelar::gateway {
/// Trying to `take_approved_call` with a wrong payload.
const EPayloadHashMismatch: u64 = 5;

/// Trying to execute the same operatorhsip transfer command again.
const EAlreadyTransferedOperatorship: u64 = 6;

/// Trying to set initial validators again
const EAlreadyInitialized: u64 = 6;

// These are currently supported
const SELECTOR_APPROVE_CONTRACT_CALL: vector<u8> = b"approveContractCall";
const SELECTOR_TRANSFER_OPERATORSHIP: vector<u8> = b"transferOperatorship";
Expand All @@ -63,6 +69,7 @@ module axelar::gateway {
id: UID,
approvals: Table<address, Approval>,
validators: AxelarValidators,
operatorship_transfers: Table<address, bool>,
}

/// CallApproval struct which can consumed only by a `Channel` object.
Expand Down Expand Up @@ -97,11 +104,17 @@ module axelar::gateway {
id: object::new(ctx),
approvals: table::new(ctx),
validators: validators::new(),
operatorship_transfers: table::new(ctx),
};

transfer::share_object(gateway);
}

public fun set_initial_validators(self: &mut Gateway, payload: vector<u8>) {
assert!(self.validators.epoch() == 0, EAlreadyInitialized);
self.validators.transfer_operatorship(payload);
}

#[allow(implicit_const_copy)]
/// The main entrypoint for the external approval processing.
/// Parses data and attaches call approvals to the Axelar object to be
Expand Down Expand Up @@ -175,6 +188,9 @@ module axelar::gateway {
if (!allow_operatorship_transfer) {
continue
};

assert!(!self.operatorship_transfers.contains(cmd_id), EAlreadyTransferedOperatorship);
self.operatorship_transfers.add(cmd_id, true);
allow_operatorship_transfer = false;
borrow_mut_validators(self).transfer_operatorship(payload);
} else {
Expand Down Expand Up @@ -283,13 +299,17 @@ module axelar::gateway {
fun borrow_mut_validators(self: &mut Gateway): &mut AxelarValidators {
&mut self.validators
}

#[test_only]
public fun new(ctx: &mut TxContext): Gateway {
let mut validators = validators::new();
validators.init_for_testing();

Gateway {
id: object::new(ctx),
approvals: table::new(ctx),
validators: validators::new(),
validators,
operatorship_transfers: table::new(ctx),
}
}

Expand Down Expand Up @@ -344,28 +364,28 @@ module axelar::gateway {
];

let data = get_data(&chain_id, &command_ids, &commands, &params);
let proof = x"";
let proof = validators::proof_for_testing();
let mut input = vector[];

vector::append(&mut input, bcs::to_bytes(&data));
vector::append(&mut input, bcs::to_bytes(&proof));

assert!(gateway.approvals.contains(@0x1) == false, 0);
assert!(gateway.validators.epoch() == 0, 3);
assert!(gateway.validators.epoch() == 1, 3);

process_commands(&mut gateway, input);

assert!(gateway.approvals.contains(@0x1) == true, 2);
let approval_hash = get_approval_hash(
&@0x1,
&@0x1,
&source_chain,
&source_address,
&target_id,
&payload_hash,
);

assert!(approval_hash == gateway.approvals.borrow(@0x1).approval_hash, 3);
assert!(gateway.validators.epoch() == 1, 4);
assert!(gateway.validators.epoch() == 2, 4);

assert!(gateway.validators.test_contains_operators(
&new_operators_1,
Expand All @@ -381,7 +401,7 @@ module axelar::gateway {
&new_operators_1,
&new_weights_1,
new_threshold_1,
) == 1, 7);
) == 2, 7);


sui::test_utils::destroy(gateway);
Expand All @@ -399,9 +419,9 @@ module axelar::gateway {
let params = vector[];

let data = get_data(&chain_id, &command_ids, &commands, &params);
let proof = x"";
let proof = validators::proof_for_testing();
let mut input = vector[];

vector::append(&mut input, bcs::to_bytes(&data));
vector::append(&mut input, bcs::to_bytes(&proof));

Expand All @@ -422,9 +442,9 @@ module axelar::gateway {
let params = vector[];

let data = get_data(&chain_id, &command_ids, &commands, &params);
let proof = x"";
let proof = validators::proof_for_testing();
let mut input = vector[];

vector::append(&mut input, bcs::to_bytes(&data));
vector::append(&mut input, bcs::to_bytes(&proof));

Expand All @@ -445,9 +465,9 @@ module axelar::gateway {
let params = vector[x""];

let data = get_data(&chain_id, &command_ids, &commands, &params);
let proof = x"";
let proof = validators::proof_for_testing();
let mut input = vector[];

vector::append(&mut input, bcs::to_bytes(&data));
vector::append(&mut input, bcs::to_bytes(&proof));

Expand Down Expand Up @@ -483,9 +503,9 @@ module axelar::gateway {
];

let data = get_data(&chain_id, &command_ids, &commands, &params);
let proof = x"";
let proof = validators::proof_for_testing();
let mut input = vector[];

vector::append(&mut input, bcs::to_bytes(&data));
vector::append(&mut input, bcs::to_bytes(&proof));

Expand Down
32 changes: 26 additions & 6 deletions move/axelar/sources/validators.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ module axelar::validators {
proof: vector<u8>
): bool {
let epoch = epoch(validators);
// Allow the validators to validate any proof before the first set of operators is set (so that they can be rotated).
if (epoch == 0) {
return true
};

// Turn everything into bcs bytes and split data.
let mut proof = bcs::new(proof);
Expand All @@ -73,7 +69,7 @@ module axelar::validators {
assert!(operators_epoch != 0 && epoch - operators_epoch < OLD_KEY_RETENTION, EInvalidOperators);
let (mut i, mut weight, mut operator_index) = (0, 0, 0);
let total_signatures = vector::length(&signatures);
while (i < total_signatures) {
while (i < total_signatures && weight < threshold) {

let mut signature = *vector::borrow(&signatures, i);
normalize_signature(&mut signature);
Expand All @@ -86,12 +82,14 @@ module axelar::validators {
assert!(operator_index < operators_length, EMalformedSigners);

weight = weight + *vector::borrow(&weights, operator_index);
if (weight >= threshold) { return operators_epoch == epoch };

operator_index = operator_index + 1;

i = i + 1;
};

if (weight >= threshold) { return operators_epoch == epoch };

abort ELowSignaturesWeight
}

Expand All @@ -118,6 +116,7 @@ module axelar::validators {
// Remove old epoch for the operators if it exists
let epoch = validators.epoch() + 1;
let epoch_for_hash = validators.epoch_for_hash_mut();
// We are likely to change the architecture a bit to conform to our standars better in the future.
if (epoch_for_hash.contains(&new_operators_hash)) {
epoch_for_hash.remove(&new_operators_hash);
};
Expand Down Expand Up @@ -191,6 +190,27 @@ module axelar::validators {
/// Expected to be returned from ecrecover.
const SIGNER: vector<u8> = x"037286a4f1177bea06c8e15cf6ec3df0b7747a01ac2329ca2999dfd74eff599028";

#[test_only]
public fun proof_for_testing(): vector<u8> {
let mut proof: vector<u8> = vector[];
let mut i = 0;
while ( i < 19 ) {
vector::push_back(&mut proof, 0);
i = i + 1;
};
proof
}

#[test_only]
public fun init_for_testing(self: &mut AxelarValidators) {
let new_operators_hash = operators_hash(&vector[], &vector[], 0);
let epoch = 1;

self.epoch_for_hash.insert(new_operators_hash, epoch);

self.set_epoch(epoch);
}

#[test_only]
public fun get_transfer_params(new_operators: &vector<vector<u8>>, new_weights: &vector<u128>, new_threshold: &u128): vector<u8> {
let mut bcs = vector::empty<u8>();
Expand Down
3 changes: 2 additions & 1 deletion move/squid/sources/squid/deepbook_v2.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ module squid::deepbook_v2 {
if(has_base) {
let (amount_left, output) = predict_base_for_quote(
pool,
// these are run in sequence before anything is done with balances, so `get_estimate` is the correct function to use.
self.coin_bag().get_estimate<T1>(),
lot_size,
clock,
Expand Down Expand Up @@ -368,4 +369,4 @@ module squid::deepbook_v2 {
vector[type_base, type_quote] ,
)
}
}
}
Loading

0 comments on commit a42642e

Please sign in to comment.