Skip to content

test: add failure-case coverage for Chainvoice invoices #142

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.

Sign up for GitHub

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

Open
tani404 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

test: add failure-case coverage for Chainvoice invoices #142

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cf616d6
test: add failure-case coverage for Chainvoice invoices
tani404 Mar 18, 2026
fcd723f
Update contracts/test/Chainvoice.t.sol
tani404 Mar 19, 2026
60512ff
Fix test to expect revert on incorrect payment amount
tani404 Mar 19, 2026
10b959e
Fix typo in variable name for fee retrieval
tani404 Mar 19, 2026
c23f639
Merge branch 'main' into test/failure-cases-chainvoice
tani404 Mar 26, 2026
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
125 changes: 61 additions & 64 deletions contracts/test/Chainvoice.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import "../src/Chainvoice.sol";


contract ChainvoiceTest is Test {
Chainvoice chainvoice;

Expand Down Expand Up @@ -51,6 +52,7 @@ contract ChainvoiceTest is Test {
assertFalse(inv.isCancelled);
}


/* ------------------------------------------------------------ */
/* PAY INVOICE */
/* ------------------------------------------------------------ */
Expand Down Expand Up @@ -93,95 +95,90 @@ contract ChainvoiceTest is Test {
}

/* ------------------------------------------------------------ */
/* FAILURE CASES */
/* FAILURE CASES - CREATE INVOICE */
/* ------------------------------------------------------------ */

function testCreateInvoiceRevertIfInvalidAddress() public{
vm.prank(alice);
vm.expectRevert("Recipient address is zero");
chainvoice.createInvoice(address(0), 1 ether, address(0), "data", "hash");
}

function testCreateInvoiceRevertIfSelfInvoicing() public{
vm.prank(alice);
vm.expectRevert("Self-invoicing not allowed");
chainvoice.createInvoice(alice, 1 ether, address(0), "data", "hash");
}
Comment on lines +101 to +111
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Tests use string reverts but contract throws custom errors — tests will fail.

The createInvoice function in Chainvoice.sol uses custom errors (revert ZeroAddress() at line 124, revert SelfInvoicing() at line 125), not string reverts. These vm.expectRevert("...") calls will not match the actual revert data.

🛠️ Proposed fix: use custom error selectors 
 function testCreateInvoiceRevertIfInvalidAddress() public{
     vm.prank(alice);
-    vm.expectRevert("Recipient address is zero");
+    vm.expectRevert(Chainvoice.ZeroAddress.selector);
     chainvoice.createInvoice(address(0), 1 ether, address(0), "data", "hash");
 }

 function testCreateInvoiceRevertIfSelfInvoicing() public{
     vm.prank(alice);
-    vm.expectRevert("Self-invoicing not allowed");
+    vm.expectRevert(Chainvoice.SelfInvoicing.selector);
     chainvoice.createInvoice(alice, 1 ether, address(0), "data", "hash");
 }

As per coding guidelines, "Ensure failure paths and revert scenarios are explicitly handled and validated."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function testCreateInvoiceRevertIfInvalidAddress() public{
vm.prank(alice);
vm.expectRevert("Recipient address is zero");
chainvoice.createInvoice(address(0), 1 ether, address(0), "data", "hash");
}
function testCreateInvoiceRevertIfSelfInvoicing() public{
vm.prank(alice);
vm.expectRevert("Self-invoicing not allowed");
chainvoice.createInvoice(alice, 1 ether, address(0), "data", "hash");
}
function testCreateInvoiceRevertIfInvalidAddress() public{
vm.prank(alice);
vm.expectRevert(Chainvoice.ZeroAddress.selector);
chainvoice.createInvoice(address(0), 1 ether, address(0), "data", "hash");
}
function testCreateInvoiceRevertIfSelfInvoicing() public{
vm.prank(alice);
vm.expectRevert(Chainvoice.SelfInvoicing.selector);
chainvoice.createInvoice(alice, 1 ether, address(0), "data", "hash");
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/test/Chainvoice.t.sol` around lines 101 - 111, The tests
testCreateInvoiceRevertIfInvalidAddress and
testCreateInvoiceRevertIfSelfInvoicing call chainvoice.createInvoice but use
string vm.expectRevert messages, while the contract reverts with custom errors
ZeroAddress() and SelfInvoicing(); update those tests to expect the custom error
selectors instead (e.g. use
vm.expectRevert(abi.encodeWithSelector(ZeroAddress.selector)) and
vm.expectRevert(abi.encodeWithSelector(SelfInvoicing.selector))) so the revert
data from Chainvoice.createInvoice matches the test expectation.


/* ------------------------------------------------------------ */
/* FAILURE CASES - PAY INVOICE */
/* ------------------------------------------------------------ */

function testPayInvoice_RevertIfWrongPayer() public {
modifier createInvoice(){
vm.prank(alice);
chainvoice.createInvoice(bob, 1 ether, address(0), "data", "hash");
_;
}

function testPayInvoice_RevertIfWrongPayer() public createInvoice {
uint256 fee = chainvoice.fee();
vm.expectRevert(Chainvoice.NotAuthorizedPayer.selector);
vm.prank(alice);
chainvoice.payInvoice{value: 1 ether + fee}(0);
}

function testPayInvoice_RevertIfIncorrectValue() public {
vm.prank(alice);
chainvoice.createInvoice(bob, 1 ether, address(0), "data", "hash");

vm.expectRevert(Chainvoice.IncorrectPaymentAmount.selector);
function testPayInvoice_RevertIfIncorrectValue() public createInvoice {
uint256 fee = chainvoice.fee();
vm.expectRevert("Incorrect payment amount");
vm.prank(bob);
chainvoice.payInvoice{value: 1 ether}(0);
chainvoice.payInvoice{value: 1 ether + fee + 1}(0);
}
Comment thread
tani404 marked this conversation as resolved.

/* ------------------------------------------------------------ */
/* OWNERSHIP MANAGEMENT */
/* ------------------------------------------------------------ */
function testPayInvoice_RevertIfInvalidInvoiceId() public createInvoice{
uint256 fee = chainvoice.fee();

function testInitiateOwnershipTransfer() public {
address newOwner = address(0xC0FFEE);

vm.prank(alice); // alice is not the owner
vm.expectRevert("Only owner can call");
chainvoice.initiateOwnershipTransfer(newOwner);

vm.prank(address(this)); // this is the owner (from setUp)
chainvoice.initiateOwnershipTransfer(newOwner);

assertEq(chainvoice.pendingOwner(), newOwner);
vm.expectRevert("Invalid invoice ID");
vm.prank(bob);
chainvoice.payInvoice{value: 1 ether + fee}(type(uint256).max);
}
Comment thread
tani404 marked this conversation as resolved.

function testInitiateOwnershipTransferInvalidAddress() public {
vm.expectRevert(Chainvoice.InvalidNewOwner.selector);
chainvoice.initiateOwnershipTransfer(address(0));
function testPayInvoice_RevertIfInvoiceAlreadyPaid() public createInvoice{
uint256 fee = chainvoice.fee();

// Try to transfer to self
vm.expectRevert(Chainvoice.InvalidNewOwner.selector);
chainvoice.initiateOwnershipTransfer(address(this));
}
vm.prank(bob);
chainvoice.payInvoice{value: 1 ether + fee}(0);

function testAcceptOwnership() public {
address newOwner = address(0xC0FFEE);

chainvoice.initiateOwnershipTransfer(newOwner);

vm.prank(newOwner);
chainvoice.acceptOwnership();

assertEq(chainvoice.owner(), newOwner);
assertEq(chainvoice.pendingOwner(), address(0));
vm.expectRevert("Already paid");
vm.prank(bob);
chainvoice.payInvoice{value: 1 ether + fee}(0);
}
Comment on lines +130 to 154
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Tests use string reverts but contract throws custom errors — tests will fail.

The payInvoice function uses custom errors (see Chainvoice.sol lines 238-276):

  • revert IncorrectPaymentAmount() not "Incorrect payment amount"
  • revert InvalidInvoiceId() not "Invalid invoice ID"
  • revert InvoiceAlreadyPaid() not "Already paid"
🛠️ Proposed fix: use custom error selectors 
 function testPayInvoice_RevertIfIncorrectValue() public createInvoice {
     uint256 fee = chainvoice.fee();
-    vm.expectRevert("Incorrect payment amount");
+    vm.expectRevert(Chainvoice.IncorrectPaymentAmount.selector);
     vm.prank(bob);
     chainvoice.payInvoice{value: 1 ether + fee + 1}(0);
 }

 function testPayInvoice_RevertIfInvalidInvoiceId() public createInvoice{
     uint256 fee = chainvoice.fee();

-    vm.expectRevert("Invalid invoice ID");
+    vm.expectRevert(Chainvoice.InvalidInvoiceId.selector);
     vm.prank(bob);
     chainvoice.payInvoice{value: 1 ether + fee}(type(uint256).max);
 }

 function testPayInvoice_RevertIfInvoiceAlreadyPaid() public createInvoice{
     uint256 fee = chainvoice.fee();

     vm.prank(bob);
     chainvoice.payInvoice{value: 1 ether + fee}(0);

-    vm.expectRevert("Already paid");
+    vm.expectRevert(Chainvoice.InvoiceAlreadyPaid.selector);
     vm.prank(bob);
     chainvoice.payInvoice{value: 1 ether + fee}(0);
 }

As per coding guidelines, "Ensure failure paths and revert scenarios are explicitly handled and validated."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/test/Chainvoice.t.sol` around lines 130 - 154, The tests currently
assert string revert messages but Chainvoice.sol throws custom errors; update
the three tests (testPayInvoice_RevertIfIncorrectValue,
testPayInvoice_RevertIfInvalidInvoiceId,
testPayInvoice_RevertIfInvoiceAlreadyPaid) to expect the custom error selectors
instead of strings—for example call
vm.expectRevert(abi.encodeWithSelector(Chainvoice.IncorrectPaymentAmount.selector))
before the failing payInvoice call, use Chainvoice.InvalidInvoiceId.selector for
the invalid ID case, and Chainvoice.InvoiceAlreadyPaid.selector for the
already-paid case so the vm expects match the contract's custom errors on
chainvoice.payInvoice calls.


Comment thread
tani404 marked this conversation as resolved.
function testAcceptOwnershipNotPending() public {
vm.prank(address(0xDEADBEEF));
vm.expectRevert(Chainvoice.OwnershipNotPending.selector);
chainvoice.acceptOwnership();
}
/* ------------------------------------------------------------ */
/* FAILURE CASES - CANCEL INVOICE */
/* ------------------------------------------------------------ */

function testCancelOwnershipTransfer() public {
address newOwner = address(0xC0FFEE);

chainvoice.initiateOwnershipTransfer(newOwner);
assertEq(chainvoice.pendingOwner(), newOwner);

chainvoice.cancelOwnershipTransfer();
assertEq(chainvoice.pendingOwner(), address(0));
function testCancelInvoiceRevertIfNotTheOwner() public createInvoice{
vm.expectRevert("Only invoice creator can cancel");
vm.prank(bob);
chainvoice.cancelInvoice(0);
}

function testCancelOwnershipTransferNoPending() public {
vm.expectRevert(Chainvoice.OwnershipNotPending.selector);
chainvoice.cancelOwnershipTransfer();
}
function testCancelInvoiceRevertIfItIsAlreadyPaid() public createInvoice{
uint256 fee = chainvoice.fee();
vm.prank(bob);
chainvoice.payInvoice{value: 1 ether + fee}(0);

function testFeeUpdateEvent() public {
uint256 newFee = 0.001 ether;
chainvoice.setFeeAmount(newFee);
assertEq(chainvoice.fee(), newFee);
vm.expectRevert("Invoice not cancellable");
vm.prank(alice);
chainvoice.cancelInvoice(0);
}

function testTreasuryAddressUpdateEvent() public {
address newTreasury = address(0xdead);
chainvoice.setTreasuryAddress(newTreasury);
assertEq(chainvoice.treasuryAddress(), newTreasury);
function testCancelInvoiceRevertIfItIsAlreadyCancelled() public createInvoice{
vm.prank(alice);
chainvoice.cancelInvoice(0);

vm.expectRevert("Invoice not cancellable");
vm.prank(alice);
chainvoice.cancelInvoice(0);
}
Comment thread
tani404 marked this conversation as resolved.
Comment on lines +160 to 183
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Tests use string reverts but contract throws custom errors — tests will fail.

The cancelInvoice function uses custom errors (see Chainvoice.sol lines 220-235):

  • revert NotInvoiceCreator() not "Only invoice creator can cancel"
  • revert InvoiceNotCancellable() not "Invoice not cancellable"
🛠️ Proposed fix: use custom error selectors 
 function testCancelInvoiceRevertIfNotTheOwner() public createInvoice{
-    vm.expectRevert("Only invoice creator can cancel");
+    vm.expectRevert(Chainvoice.NotInvoiceCreator.selector);
     vm.prank(bob);
     chainvoice.cancelInvoice(0);
 }

 function testCancelInvoiceRevertIfItIsAlreadyPaid() public createInvoice{
     uint256 fee = chainvoice.fee();
     vm.prank(bob);
     chainvoice.payInvoice{value: 1 ether + fee}(0);

-    vm.expectRevert("Invoice not cancellable");
+    vm.expectRevert(Chainvoice.InvoiceNotCancellable.selector);
     vm.prank(alice);
     chainvoice.cancelInvoice(0);
 }

 function testCancelInvoiceRevertIfItIsAlreadyCancelled() public createInvoice{
     vm.prank(alice);
     chainvoice.cancelInvoice(0);

-    vm.expectRevert("Invoice not cancellable");
+    vm.expectRevert(Chainvoice.InvoiceNotCancellable.selector);
     vm.prank(alice);
     chainvoice.cancelInvoice(0);
 }

As per coding guidelines, "Ensure failure paths and revert scenarios are explicitly handled and validated."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/test/Chainvoice.t.sol` around lines 160 - 183, Tests call
cancelInvoice in testCancelInvoiceRevertIfNotTheOwner,
testCancelInvoiceRevertIfItIsAlreadyPaid, and
testCancelInvoiceRevertIfItIsAlreadyCancelled but assert string messages; update
them to expect the contract's custom errors instead. Replace
vm.expectRevert("Only invoice creator can cancel") with expectRevert using the
NotInvoiceCreator error selector from the Chainvoice contract, and replace
vm.expectRevert("Invoice not cancellable") with expectRevert using the
InvoiceNotCancellable error selector; adjust the three tests
(testCancelInvoiceRevertIfNotTheOwner, testCancelInvoiceRevertIfItIsAlreadyPaid,
testCancelInvoiceRevertIfItIsAlreadyCancelled) to use abi-encoded selectors (or
the test framework's custom-error expect helper) referencing
Chainvoice.NotInvoiceCreator.selector and
Chainvoice.InvoiceNotCancellable.selector.

}
Loading