Skip to content

Commit

Permalink
Merge pull request #52 from argentlabs/align-error-message
Browse files Browse the repository at this point in the history 
Align error message
  • Loading branch information
@Leonard-Pat
Leonard-Pat authored Jun 27, 2024
2 parents 40b4f79 + cc94637 commit cf1aea4
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 56 deletions.
41 changes: 20 additions & 21 deletions src/contracts/escrow_account.cairo
View file
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
View file
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
View file
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
14 changes: 7 additions & 7 deletions tests-integration/account.test.ts
View file
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 @@ -37,7 +37,7 @@ describe("Escrow Account", function () {
const { gift } = await defaultDepositTestSetup({ factory });
const minimalCallData = CallData.compile([buildGiftCallData(gift)]);

await expectRevertWithErrorMessage("gift/invalid-selector", () =>
await expectRevertWithErrorMessage("escr-lib/invalid-selector", () =>
deployer.execute(executeActionOnAccount("claim_internal", calculateEscrowAddress(gift), minimalCallData)),
);
});
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
View file
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
View file
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
View file
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
Loading

0 comments on commit cf1aea4

Please sign in to comment.