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

Align error message #52

Merged
merged 6 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
41 changes: 20 additions & 21 deletions src/contracts/escrow_account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -76,55 +76,54 @@ mod EscrowAccount {
impl IAccountImpl of IAccount<ContractState> {
fn __validate__(ref self: ContractState, calls: Array<Call>) -> felt252 {
let execution_info = get_execution_info().unbox();
assert(execution_info.caller_address.is_zero(), 'gift-acc/only-protocol');
assert(calls.len() == 1, 'gift-acc/invalid-call-len');
assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol');
assert(calls.len() == 1, 'escrow/invalid-call-len');
let Call { to, selector, calldata } = calls.at(0);
assert(*to == get_contract_address(), 'gift-acc/invalid-call-to');
assert(*selector == selector!("claim_internal"), 'gift-acc/invalid-call-selector');
let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata)
.expect('gift-acc/invalid-calldata');
assert(*to == get_contract_address(), 'escrow/invalid-call-to');
assert(*selector == selector!("claim_internal"), 'escrow/invalid-call-selector');
let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata).expect('escrow/invalid-calldata');
assert_valid_claim(gift);

let tx_info = execution_info.tx_info.unbox();
assert(tx_info.nonce == 0, 'gift-acc/invalid-gift-nonce');
assert(tx_info.nonce == 0, 'escrow/invalid-gift-nonce');
let execution_hash = tx_info.transaction_hash;
let signature = tx_info.signature;
assert(signature.len() == 2, 'gift-acc/invalid-signature-len');
assert(signature.len() == 2, 'escrow/invalid-signature-len');

let tx_version = tx_info.version;
assert(
check_ecdsa_signature(execution_hash, gift.gift_pubkey, *signature[0], *signature[1])
|| tx_version == TX_V3_ESTIMATE
|| tx_version == TX_V1_ESTIMATE,
'invalid-signature'
'escrow/invalid-signature'
);
if gift.fee_token == STRK_ADDRESS() {
assert(tx_version == TX_V3 || tx_version == TX_V3_ESTIMATE, 'gift-acc/invalid-tx3-version');
assert(tx_version == TX_V3 || tx_version == TX_V3_ESTIMATE, 'escrow/invalid-tx3-version');
let tx_fee = compute_max_fee_v3(tx_info, tx_info.tip);
assert(tx_fee <= gift.fee_amount, 'gift-acc/max-fee-too-high-v3');
assert(tx_fee <= gift.fee_amount, 'escrow/max-fee-too-high-v3');
} else if gift.fee_token == ETH_ADDRESS() {
assert(tx_version == TX_V1 || tx_version == TX_V1_ESTIMATE, 'gift-acc/invalid-tx1-version');
assert(tx_info.max_fee <= gift.fee_amount, 'gift-acc/max-fee-too-high-v1');
assert(tx_version == TX_V1 || tx_version == TX_V1_ESTIMATE, 'escrow/invalid-tx1-version');
assert(tx_info.max_fee <= gift.fee_amount, 'escrow/max-fee-too-high-v1');
} else {
core::panic_with_felt252('gift-acc/invalid-token');
core::panic_with_felt252('escrow/invalid-token-fee');
}
VALIDATED
}

fn __execute__(ref self: ContractState, calls: Array<Call>) -> Array<Span<felt252>> {
let execution_info = get_execution_info().unbox();
assert(execution_info.caller_address.is_zero(), 'gift-acc/only-protocol');
assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol');
let tx_version = execution_info.tx_info.unbox().version;
assert(
tx_version == TX_V3
|| tx_version == TX_V1
|| tx_version == TX_V3_ESTIMATE
|| tx_version == TX_V1_ESTIMATE,
'gift-acc/invalid-tx-version'
'escrow/invalid-tx-version'
);
let Call { .., calldata }: @Call = calls[0];
let (gift, receiver): (GiftData, ContractAddress) = full_deserialize(*calldata)
.expect('gift-acc/invalid-calldata');
.expect('escrow/invalid-calldata');
// The __validate__ function already ensures the claim is valid
let library_class_hash: ClassHash = IGiftFactoryDispatcher { contract_address: gift.factory }
.get_escrow_lib_class_hash(gift.escrow_class_hash);
Expand All @@ -133,7 +132,7 @@ mod EscrowAccount {

fn is_valid_signature(self: @ContractState, hash: felt252, signature: Array<felt252>) -> felt252 {
let mut signature_span = signature.span();
let gift: GiftData = Serde::deserialize(ref signature_span).expect('gift-acc/invalid-gift');
let gift: GiftData = Serde::deserialize(ref signature_span).expect('escrow/invalid-gift');
get_validated_lib(gift).is_valid_account_signature(gift, hash, signature_span)
}

Expand All @@ -148,7 +147,7 @@ mod EscrowAccount {
impl GiftAccountImpl of IEscrowAccount<ContractState> {
fn execute_action(ref self: ContractState, selector: felt252, calldata: Array<felt252>) -> Span<felt252> {
let mut calldata_span = calldata.span();
let gift: GiftData = Serde::deserialize(ref calldata_span).expect('gift-acc/invalid-gift');
let gift: GiftData = Serde::deserialize(ref calldata_span).expect('escrow/invalid-gift');
let lib = get_validated_lib(gift);
lib.execute_action(lib.class_hash, selector, calldata.span())
}
Expand All @@ -159,7 +158,7 @@ mod EscrowAccount {
fn execute_from_outside_v2(
ref self: ContractState, outside_execution: OutsideExecution, mut signature: Span<felt252>
) -> Array<Span<felt252>> {
let gift: GiftData = Serde::deserialize(ref signature).expect('gift-acc/invalid-gift');
let gift: GiftData = Serde::deserialize(ref signature).expect('escrow/invalid-gift');
get_validated_lib(gift).execute_from_outside_v2(gift, outside_execution, signature)
}

Expand All @@ -177,7 +176,7 @@ mod EscrowAccount {

fn assert_valid_claim(gift: GiftData) {
let calculated_address = calculate_escrow_account_address(gift);
assert(calculated_address == get_contract_address(), 'gift-acc/invalid-escrow-address');
assert(calculated_address == get_contract_address(), 'escrow/invalid-escrow-address');
}

fn compute_max_fee_v3(tx_info: TxInfo, tip: u128) -> u128 {
Expand Down
23 changes: 11 additions & 12 deletions src/contracts/escrow_library.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ mod EscrowLibrary {
// This prevents creating instances of this classhash by mistake, as it's not needed.
// While it is technically possible to create instances by replacing classhashes, this practice is not recommended.
// This contract is intended to be used exclusively through library calls.
panic_with_felt252('instances-not-recommended')
panic_with_felt252('escr-lib/instance-not-recommend')
}

#[abi(embed_v0)]
Expand All @@ -96,7 +96,7 @@ mod EscrowLibrary {
let is_whitelisted = selector == selector!("claim_external")
|| selector == selector!("claim_dust")
|| selector == selector!("cancel");
assert(is_whitelisted, 'gift/invalid-selector');
assert(is_whitelisted, 'escr-lib/invalid-selector');
library_call_syscall(this_class_hash, selector, args).unwrap()
}

Expand All @@ -111,17 +111,16 @@ mod EscrowLibrary {
.get_message_hash_rev_1(get_contract_address());
assert(
check_ecdsa_signature(claim_external_hash, gift.gift_pubkey, signature.r, signature.s),
'gift/invalid-ext-signature'
'escr-lib/invalid-ext-signature'
);
self.proceed_with_claim(gift, receiver, dust_receiver);
}

fn cancel(ref self: ContractState, gift: GiftData) {
let contract_address = get_contract_address();
assert(get_caller_address() == gift.sender, 'gift/wrong-sender');

assert(get_caller_address() == gift.sender, 'escr-lib/wrong-sender');
let gift_balance = balance_of(gift.gift_token, contract_address);
assert(gift_balance > 0, 'gift/already-claimed');
assert(gift_balance > 0, 'escr-lib/already-claimed');
if gift.gift_token == gift.fee_token {
// Sender also gets the dust
transfer_from_account(gift.gift_token, gift.sender, gift_balance);
Expand All @@ -137,9 +136,9 @@ mod EscrowLibrary {
fn claim_dust(ref self: ContractState, gift: GiftData, receiver: ContractAddress) {
let contract_address = get_contract_address();
let factory_owner = IOwnableDispatcher { contract_address: gift.factory }.owner();
assert(factory_owner == get_caller_address(), 'gift/only-factory-owner');
assert(factory_owner == get_caller_address(), 'escr-lib/only-factory-owner');
let gift_balance = balance_of(gift.gift_token, contract_address);
assert(gift_balance < gift.gift_amount, 'gift/not-yet-claimed');
assert(gift_balance < gift.gift_amount, 'escr-lib/not-yet-claimed');
if gift.gift_token == gift.fee_token {
transfer_from_account(gift.gift_token, receiver, gift_balance);
} else {
Expand All @@ -157,7 +156,7 @@ mod EscrowLibrary {
fn execute_from_outside_v2(
ref self: ContractState, gift: GiftData, outside_execution: OutsideExecution, signature: Span<felt252>
) -> Array<Span<felt252>> {
panic_with_felt252('not-allowed-yet')
panic_with_felt252('escr-lib/not-allowed-yet')
}
}

Expand All @@ -166,10 +165,10 @@ mod EscrowLibrary {
fn proceed_with_claim(
ref self: ContractState, gift: GiftData, receiver: ContractAddress, dust_receiver: ContractAddress
) {
assert(receiver.is_non_zero(), 'gift/zero-receiver');
assert(receiver.is_non_zero(), 'escr-lib/zero-receiver');
let contract_address = get_contract_address();
let gift_balance = balance_of(gift.gift_token, contract_address);
assert(gift_balance >= gift.gift_amount, 'gift/already-claimed-or-cancel');
assert(gift_balance >= gift.gift_amount, 'escr-lib/claimed-or-cancel');

// could be optimized to 1 transfer only when the receiver is also the dust receiver,
// and the fee token is the same as the gift token
Expand All @@ -194,7 +193,7 @@ mod EscrowLibrary {
}

fn transfer_from_account(token: ContractAddress, receiver: ContractAddress, amount: u256,) {
assert(IERC20Dispatcher { contract_address: token }.transfer(receiver, amount), 'gift/transfer-failed');
assert(IERC20Dispatcher { contract_address: token }.transfer(receiver, amount), 'escr-lib/transfer-failed');
}

fn balance_of(token: ContractAddress, account: ContractAddress) -> u256 {
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/gift_factory.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ mod GiftFactory {
// This should do some sanity checks
// We should check that the new implementation is a valid implementation
// Execute the upgrade using replace_class_syscall(...)
panic_with_felt252('downgrade-not-allowed');
panic_with_felt252('gift-fac/downgrade-not-allowed');
}
}

Expand Down
12 changes: 6 additions & 6 deletions tests-integration/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("Escrow Account", function () {
const { gift } = await defaultDepositTestSetup({ factory });
const escrowAddress = calculateEscrowAddress(gift);

await expectRevertWithErrorMessage("gift-acc/only-protocol", () =>
await expectRevertWithErrorMessage("escrow/only-protocol", () =>
deployer.execute([{ contractAddress: escrowAddress, calldata: [0x0], entrypoint: "__validate__" }]),
);
});
Expand All @@ -27,7 +27,7 @@ describe("Escrow Account", function () {
const { gift } = await defaultDepositTestSetup({ factory });
const escrowAddress = calculateEscrowAddress(gift);

await expectRevertWithErrorMessage("gift-acc/only-protocol", () =>
await expectRevertWithErrorMessage("escrow/only-protocol", () =>
deployer.execute([{ contractAddress: escrowAddress, calldata: [0x0], entrypoint: "__execute__" }]),
);
});
Expand All @@ -47,7 +47,7 @@ describe("Escrow Account", function () {
const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory });
const receiver = randomReceiver();

await expectRevertWithErrorMessage("gift-acc/invalid-call-to", () =>
await expectRevertWithErrorMessage("escrow/invalid-call-to", () =>
claimInternal({
gift,
receiver,
Expand All @@ -64,7 +64,7 @@ describe("Escrow Account", function () {

const escrowAccount = getEscrowAccount(gift, giftPrivateKey);

await expectRevertWithErrorMessage("gift-acc/invalid-call-selector", () =>
await expectRevertWithErrorMessage("escrow/invalid-call-selector", () =>
escrowAccount.execute(
[{ contractAddress: escrowAccount.address, calldata: [], entrypoint: "execute_action" }],
undefined,
Expand All @@ -77,7 +77,7 @@ describe("Escrow Account", function () {
const { factory } = await setupGiftProtocol();
const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory });
const escrowAccount = getEscrowAccount(gift, giftPrivateKey);
await expectRevertWithErrorMessage("gift-acc/invalid-call-len", () =>
await expectRevertWithErrorMessage("escrow/invalid-call-len", () =>
escrowAccount.execute(
[
{ contractAddress: escrowAccount.address, calldata: [], entrypoint: "execute_action" },
Expand All @@ -96,7 +96,7 @@ describe("Escrow Account", function () {

// double claim
await claimInternal({ gift, receiver, giftPrivateKey: giftPrivateKey });
await expectRevertWithErrorMessage("gift-acc/invalid-gift-nonce", () =>
await expectRevertWithErrorMessage("escrow/invalid-gift-nonce", () =>
claimInternal({ gift, receiver, giftPrivateKey: giftPrivateKey, details: { skipValidate: false } }),
);
});
Expand Down
12 changes: 7 additions & 5 deletions tests-integration/cancel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("Cancel Gift", function () {
// Check balance gift address address == 0
await manager.tokens.tokenBalance(escrowAddress, gift.fee_token).should.eventually.equal(0n);

await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () =>
await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () =>
claimInternal({ gift, receiver, giftPrivateKey }),
);
});
Expand Down Expand Up @@ -62,15 +62,17 @@ describe("Cancel Gift", function () {
await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n);
await manager.tokens.tokenBalance(escrowAddress, gift.fee_token).should.eventually.equal(0n);

await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () =>
await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () =>
claimInternal({ gift, receiver, giftPrivateKey }),
);
});

it(`wrong sender`, async function () {
const { factory } = await setupGiftProtocol();
const { gift } = await defaultDepositTestSetup({ factory });
await expectRevertWithErrorMessage("gift/wrong-sender", () => cancelGift({ gift, senderAccount: devnetAccount() }));
await expectRevertWithErrorMessage("escr-lib/wrong-sender", () =>
cancelGift({ gift, senderAccount: devnetAccount() }),
);
});

it(`owner reclaim dust (gift_token == fee_token)`, async function () {
Expand Down Expand Up @@ -99,7 +101,7 @@ describe("Cancel Gift", function () {
await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n);
});

it(`gift/already-claimed (gift_token != fee_token)`, async function () {
it(`escr-lib/already-claimed (gift_token != fee_token)`, async function () {
const mockERC20 = await deployMockERC20();
const { factory } = await setupGiftProtocol();
const { gift, giftPrivateKey } = await defaultDepositTestSetup({
Expand All @@ -109,6 +111,6 @@ describe("Cancel Gift", function () {
const receiver = randomReceiver();

await claimInternal({ gift, receiver, giftPrivateKey });
await expectRevertWithErrorMessage("gift/already-claimed", () => cancelGift({ gift }));
await expectRevertWithErrorMessage("escr-lib/already-claimed", () => cancelGift({ gift }));
});
});
10 changes: 6 additions & 4 deletions tests-integration/claim_external.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ describe("Claim External", function () {
const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory });
const receiver = "0x0";

await expectRevertWithErrorMessage("gift/zero-receiver", () => claimExternal({ gift, receiver, giftPrivateKey }));
await expectRevertWithErrorMessage("escr-lib/zero-receiver", () =>
claimExternal({ gift, receiver, giftPrivateKey }),
);
});

it(`Cannot call claim external twice`, async function () {
Expand All @@ -97,7 +99,7 @@ describe("Claim External", function () {
const receiver = randomReceiver();

await claimExternal({ gift, receiver, giftPrivateKey });
await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () =>
await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () =>
claimExternal({ gift, receiver, giftPrivateKey }),
);
});
Expand All @@ -106,7 +108,7 @@ describe("Claim External", function () {
const { factory } = await setupGiftProtocol();
const { gift } = await defaultDepositTestSetup({ factory });
const receiver = randomReceiver();
await expectRevertWithErrorMessage("gift/invalid-ext-signature", () =>
await expectRevertWithErrorMessage("escr-lib/invalid-ext-signature", () =>
claimExternal({ gift: gift, receiver, giftPrivateKey: "0x1234" }),
);
});
Expand All @@ -127,7 +129,7 @@ describe("Claim External", function () {
// Check balance gift address address == 0
await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n);

await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () =>
await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () =>
claimExternal({ gift, receiver, giftPrivateKey }),
);
});
Expand Down
8 changes: 4 additions & 4 deletions tests-integration/claim_internal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe("Claim Internal", function () {
overrides: { feeAmount: 0n },
});

const errorMsg = useTxV3 ? "gift-acc/max-fee-too-high-v3" : "gift-acc/max-fee-too-high-v1";
const errorMsg = useTxV3 ? "escrow/max-fee-too-high-v3" : "escrow/max-fee-too-high-v1";
await expectRevertWithErrorMessage(errorMsg, () => claimInternal({ gift, receiver, giftPrivateKey }));
});

Expand All @@ -59,7 +59,7 @@ describe("Claim Internal", function () {
max_price_per_unit: num.toHexString(gasPrice),
},
};
await expectRevertWithErrorMessage("gift-acc/max-fee-too-high-v3", () =>
await expectRevertWithErrorMessage("escrow/max-fee-too-high-v3", () =>
claimInternal({
gift,
receiver,
Expand All @@ -68,7 +68,7 @@ describe("Claim Internal", function () {
}),
);
} else {
await expectRevertWithErrorMessage("gift-acc/max-fee-too-high-v1", () =>
await expectRevertWithErrorMessage("escrow/max-fee-too-high-v1", () =>
claimInternal({
gift,
receiver,
Expand All @@ -88,7 +88,7 @@ describe("Claim Internal", function () {
const receiver = randomReceiver();

await claimInternal({ gift, receiver, giftPrivateKey });
await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () =>
await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () =>
claimInternal({ gift, receiver, giftPrivateKey }),
);
});
Expand Down
2 changes: 1 addition & 1 deletion tests-integration/factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe("Test Core Factory Functions", function () {
const { gift } = await defaultDepositTestSetup({ factory });
const dustReceiver = randomReceiver();

await expectRevertWithErrorMessage("gift/only-factory-owner", () =>
await expectRevertWithErrorMessage("escr-lib/only-factory-owner", () =>
claimDust({ gift, receiver: dustReceiver, factoryOwner: devnetAccount() }),
);
});
Expand Down
Loading
Loading