Reserve Auction v1.0: add tests when the protocol fee is non-zero

[almndbtr]

Description

Towards #122 as a part of the Community Audit.

Motivation and Context

When an auction is settled, there are payouts to these respective parties:

• Royalty Payout
• Protocol Fee (optional)
• Finders Fee (optional)

The integration tests cover the Royalty Payout case and the Optional Finders fee, but not the optional Protocol Fee.

This adds new assertions to the Reserve Auction v1.0 when the protocol fee is 1 basis point (bps), the equivalent of 0.01%:

v3/contracts/test/modules/ReserveAuction/V1/ReserveAuction.integration.t.sol

Lines 82 to 84 in dfba6b3

	// Set fee parameters for Reserve Auction v1.0
	vm.prank(address(registrar));
	ZPFS.setFeeParams(address(auctions), address(protocolFeeRecipient), 1);

I chose this number based on the existing assertions defined in test/auxiliary/ZoraProtocolFeeSettings.t.sol.

How has this been tested?

In my development environment, I ran yarn test and confirmed all tests pass before opening this PR.

Checklist:

• The module includes tests written for Foundry
• The module is documented with NATSPEC
• The documentation includes UML Diagrams for external and public functions
• The module is a Hyperstructure

Reviewer Notes 👓

👋 @kulkarohan -- I'd like your evaluative feedback on this approach:

• Are there any other edge cases that we should test for, that I missed?
• Are there any stylistic changes that the linter didn't pick up that should be implemented?

Please let me know: I'm open to make any additional changes based on your input. Thank you! 🙇‍♂️

[jgeary]

this looks great @almndbtr. i cloned your repo and ran forge test (your description mistakenly says yarn) and can confirm they pass.

this is a necessary addition that we should get into the repo asap and i cant think of any relevant low hanging fruit to add, so let's go ahead and merge. @kulkarohan @tbtstl either of you want to take a glance and merge if nothing comes up?

[almndbtr]

this looks great @almndbtr. i cloned your repo and ran forge test (your description mistakenly says yarn) and can confirm they pass.

@jgeary - Thank you! 😌 If I'm reading the package.json manifest's scripts.test correctly, I think that they are logically equivalent: yarn test or npm test is an alias to forge test, as you've done:

v3/package.json

Line 39 in 479e45c

"test": "forge test",

this is a necessary addition that we should get into the repo asap and i cant think of any relevant low hanging fruit to add, so let's go ahead and merge.

I appreciate that and you advocating this go forward. I'd also like to gently share that this has another "sibling" PR ready to go for Collection Offers v1.0, too 🙇‍♂️ -- #144

[jgeary]

@almndbtr ah of course. for some reason yarn test didnt work for me at first but just confirmed it does.

[kulkarohan]

lgtm!