Skip to content
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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions contracts/adl_handler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ pub enum Error {
NotProfitable = 6,
MarketPaused = 9,
PositionNotFound = 7,
/// Max PnL factor for ADL is not configured (0) for the requested market/side.
/// Callers must set a non-zero value via DataStore before ADL can be evaluated.
MissingMaxPnlConfig = 8,
/// Issue #643: update_oracle called with the current oracle address
/// (no-op) or with this contract's own address / another of its stored
/// instance addresses (order_handler, data_store, role_store, admin) —
Expand Down
5 changes: 4 additions & 1 deletion contracts/oracle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub enum Error {
InvalidPrice = 4, // min > max or zero
StalePrice = 5, // timestamp too old
PriceNotFound = 6,
InvalidSignature = 7,
/// clear_prices called with more than MAX_CLEAR_PRICES_BATCH_SIZE tokens (issue #619).
BatchSizeLimitExceeded = 9,
}
Expand Down Expand Up @@ -273,6 +272,10 @@ impl Oracle {
);
let pubkey = get_keeper_pubkey(&env, &data_store, sp.keeper_index);
// ed25519_verify takes (&BytesN<32> pubkey, &Bytes message, &BytesN<64> sig)
// and panics with a host-level crypto error on bad signature.
// There is no SDK try_ helper (soroban-sdk 25) to convert this into a
// contract Error::InvalidSignature, so the dead variant was removed
// (#576); callers see the host error instead of a typed contract error.
env.crypto().ed25519_verify(&pubkey, &msg, &sp.signature);

// Check circuit breaker before overwriting price
Expand Down
139 changes: 139 additions & 0 deletions contracts/role_store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,4 +575,143 @@ mod tests {
))
);
}

// ── Issue #564: revoke-side RoleMemberCount/RoleMembers coverage ─────────

/// Grant a role to two accounts, revoke one, and assert the count
/// decremented by exactly one and the remaining member list is correct.
#[test]
fn test_revoke_decrements_count_and_removes_member() {
let (env, admin, contract_id) = setup();
let client = RoleStoreClient::new(&env, &contract_id);
let ctrl = roles::controller(&env);
let k1 = Address::generate(&env);
let k2 = Address::generate(&env);

client.grant_role(&admin, &k1, &ctrl);
client.grant_role(&admin, &k2, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 2);
let members = client.get_role_members(&ctrl, &0, &10);
assert_eq!(members.len(), 2);

client.revoke_role(&admin, &k1, &ctrl);

assert_eq!(
client.get_role_member_count(&ctrl),
1,
"count must decrement by exactly one after revoke"
);
assert!(!client.has_role(&k1, &ctrl));
assert!(client.has_role(&k2, &ctrl));

let members = client.get_role_members(&ctrl, &0, &10);
assert_eq!(members.len(), 1);
assert_eq!(members.get_unchecked(0), k2);
// revoked account must no longer appear
assert!(!vec_contains_b32(&client.get_roles(&k1), &ctrl));
}

/// Revoke one member out of three and assert the other two remain in order
/// and vec_remove_addr removed only the target (middle of the list).
#[test]
fn test_revoke_one_of_many_leaves_others() {
let (env, admin, contract_id) = setup();
let client = RoleStoreClient::new(&env, &contract_id);
let ctrl = roles::controller(&env);
let k1 = Address::generate(&env);
let k2 = Address::generate(&env);
let k3 = Address::generate(&env);

client.grant_role(&admin, &k1, &ctrl);
client.grant_role(&admin, &k2, &ctrl);
client.grant_role(&admin, &k3, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 3);

// revoke the middle member (k2) — exercises vec_remove_addr not just popping tail
client.revoke_role(&admin, &k2, &ctrl);

assert_eq!(client.get_role_member_count(&ctrl), 2);
assert!(!client.has_role(&k2, &ctrl));
assert!(client.has_role(&k1, &ctrl));
assert!(client.has_role(&k3, &ctrl));

let members = client.get_role_members(&ctrl, &0, &10);
assert_eq!(members.len(), 2);
// remaining members must be k1 and k3 (k2 removed from middle)
assert_eq!(members.get_unchecked(0), k1);
assert_eq!(members.get_unchecked(1), k3);

// revoke the first member (k1) — exercises remove from start of list
client.revoke_role(&admin, &k1, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 1);
let members = client.get_role_members(&ctrl, &0, &10);
assert_eq!(members.len(), 1);
assert_eq!(members.get_unchecked(0), k3);
}

/// Second revoke of the same account/role must be a harmless no-op
/// (internal_revoke_role is documented as idempotent). Count must not
/// go negative or decrement twice.
#[test]
fn test_idempotent_revoke() {
let (env, admin, contract_id) = setup();
let client = RoleStoreClient::new(&env, &contract_id);
let ctrl = roles::controller(&env);
let keeper = Address::generate(&env);

client.grant_role(&admin, &keeper, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 1);

client.revoke_role(&admin, &keeper, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 0);
assert!(!client.has_role(&keeper, &ctrl));

// second revoke — must not panic, must not decrement below zero
client.revoke_role(&admin, &keeper, &ctrl);
assert_eq!(
client.get_role_member_count(&ctrl),
0,
"second revoke must be a no-op and not go negative"
);
assert!(!client.has_role(&keeper, &ctrl));
let members = client.get_role_members(&ctrl, &0, &10);
assert_eq!(members.len(), 0);

// revoking an account that never held the role at all must also be a no-op
let never_holder = Address::generate(&env);
client.revoke_role(&admin, &never_holder, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 0);
}

/// Revoke-side bookkeeping for get_roles: after revoke, the account's
/// role list must no longer contain the revoked role while other roles
/// remain — mirroring get_roles_reflects_grants_and_revokes.
#[test]
fn test_revoke_updates_account_roles_and_member_list_together() {
let (env, admin, contract_id) = setup();
let client = RoleStoreClient::new(&env, &contract_id);
let ctrl = roles::controller(&env);
let order_keeper = roles::order_keeper(&env);
let user = Address::generate(&env);
let other = Address::generate(&env);

// give user two roles and other one role
client.grant_role(&admin, &user, &ctrl);
client.grant_role(&admin, &user, &order_keeper);
client.grant_role(&admin, &other, &ctrl);
assert_eq!(client.get_role_member_count(&ctrl), 2);

// revoke ctrl from user — user should keep order_keeper, other keeps ctrl
client.revoke_role(&admin, &user, &ctrl);

assert_eq!(client.get_role_member_count(&ctrl), 1);
let members = client.get_role_members(&ctrl, &0, &10);
assert_eq!(members.len(), 1);
assert_eq!(members.get_unchecked(0), other);

let user_roles = client.get_roles(&user);
assert_eq!(user_roles.len(), 1);
assert_eq!(user_roles.get_unchecked(0), order_keeper);
assert!(!vec_contains_b32(&user_roles, &ctrl));
}
}
1 change: 1 addition & 0 deletions contracts/test_faucet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ crate-type = ["cdylib", "lib"]

[dependencies]
soroban-sdk = { workspace = true }
gmx-keys = { path = "../../libs/keys" }

[dev-dependencies]
soroban-sdk = { workspace = true, features = ["testutils"] }
Expand Down
19 changes: 3 additions & 16 deletions contracts/test_faucet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,9 @@

use soroban_sdk::{
contract, contractclient, contracterror, contractimpl, contracttype, panic_with_error,
symbol_short, Address, BytesN, Env, Vec,
symbol_short, Address, Env, Vec,
};

/// `network_id` (SHA-256 of the network passphrase) for the Stellar public
/// network. Test faucets must never be initialized here (issue #400).
const MAINNET_NETWORK_ID: [u8; 32] = [
0x7a, 0xc3, 0x39, 0x97, 0x54, 0x4e, 0x31, 0x75, 0xd2, 0x66, 0xbd, 0x02, 0x24, 0x39, 0xb2, 0x2c,
0xdb, 0x16, 0x50, 0x8c, 0x01, 0x16, 0x3f, 0x26, 0xe5, 0xcb, 0x2a, 0x3e, 0x10, 0x45, 0xa9, 0x79,
];

#[allow(dead_code)]
#[contractclient(name = "TestTokenClient")]
trait ITestToken {
Expand Down Expand Up @@ -60,7 +53,7 @@ pub struct TestFaucet;
#[contractimpl]
impl TestFaucet {
pub fn initialize(env: Env, admin: Address, cooldown_ledgers: u32) {
require_not_mainnet(&env);
gmx_keys::require_not_mainnet(&env, Error::MainnetNotAllowed as u32);
// Issue #612: require the admin's own auth so a front-runner who
// observes the deploy transaction (or predicts the deterministic
// contract address) can't call initialize first with themselves as
Expand Down Expand Up @@ -179,12 +172,6 @@ fn do_claim(env: &Env, account: &Address, token: Address) -> i128 {
amount
}

fn require_not_mainnet(env: &Env) {
if env.ledger().network_id() == BytesN::from_array(env, &MAINNET_NETWORK_ID) {
panic_with_error!(env, Error::MainnetNotAllowed);
}
}

fn get_admin(env: &Env) -> Address {
env.storage()
.instance()
Expand Down Expand Up @@ -355,7 +342,7 @@ mod tests {
fn initialize_rejects_mainnet_network_id() {
let env = Env::default();
env.mock_all_auths();
env.ledger().set_network_id(MAINNET_NETWORK_ID);
env.ledger().set_network_id(gmx_keys::MAINNET_NETWORK_ID);
let admin = Address::generate(&env);
let faucet_id = env.register(TestFaucet, ());
TestFaucetClient::new(&env, &faucet_id).initialize(&admin, &10);
Expand Down
1 change: 1 addition & 0 deletions contracts/test_token/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ crate-type = ["cdylib", "lib"]

[dependencies]
soroban-sdk = { workspace = true }
gmx-keys = { path = "../../libs/keys" }

[dev-dependencies]
soroban-sdk = { workspace = true, features = ["testutils"] }
Loading