Omniscia Boson Protocol Audit

OfferBase Code Style Findings

OfferBase Code Style Findings

OBE-01C: Redundant Conditional Validation

Description:

The referenced conditional will redundantly evaluate whether the OfferCreator enum of the Offer structure is a Buyer even though the enum supports two states and one of the two has already been excluded.

Example:

contracts/protocol/bases/OfferBase.sol
104if (_offer.creator == OfferCreator.Seller) {
105 if (_offer.buyerId != 0) revert InvalidBuyerOfferFields();
106
107 // Validate caller is seller assistant
108 if (_authenticate) {
109 (bool isAssistant, uint256 sellerId) = getSellerIdByAssistant(sender);
110 if (!isAssistant) {
111 revert NotAssistant();
112 }
113 _offer.sellerId = sellerId;
114 }
115} else if (_offer.creator == OfferCreator.Buyer) {

Recommendation:

We advise the referenced conditional to be set to an else clause, optimizing its gas cost.

Alleviation (efd5d1a8f23c3bca7c25273ea4c912a367250119):

The Boson Protocol team evaluated this exhibit and opted to retain the code as is as they consider it to be more legible and future-proof.

OBE-02C: Unspecified Revert Conditions

TypeSeverityLocation
Code StyleOfferBase.sol:L122, L123

Description:

The referenced two conditions represent revert cases that are undocumented.

Example:

contracts/protocol/bases/OfferBase.sol
50/**
51 * @notice Creates offer. Can be reused among different facets.
52 *
53 * Emits an OfferCreated event if successful.
54 *
55 * Reverts if:
56 * - Caller is not an assistant in case of seller-initiated offer
57 * - sellerId is not 0 when buyer-initiated offer is created
58 * - collectionIndex is not 0 when buyer-initiated offer is created
59 * - royaltyInfo is not empty when buyer-initiated offer is created
60 * - priceType is not Static when buyer-initiated offer is created
61 * - Invalid offer creator value specified (OfferCreator.Seller or OfferCreator.Buyer)
62 * - Valid from date is greater than valid until date
63 * - Valid until date is not in the future
64 * - Both voucher expiration date and voucher expiration period are defined
65 * - Neither of voucher expiration date and voucher expiration period are defined
66 * - Voucher redeemable period is fixed, but it ends before it starts
67 * - Voucher redeemable period is fixed, but it ends before offer expires
68 * - Dispute period is less than minimum dispute period
69 * - Resolution period is not between the minimum and the maximum resolution period
70 * - Voided is set to true
71 * - Available quantity is set to zero
72 * - Dispute resolver wallet is not registered, except for absolute zero offers with unspecified dispute resolver
73 * - Dispute resolver is not active, except for absolute zero offers with unspecified dispute resolver
74 * - Seller is not on dispute resolver's seller allow list
75 * - Dispute resolver does not accept fees in the exchange token
76 * - Buyer cancel penalty is greater than price
77 * - When agent id is non zero:
78 * - If Agent does not exist
79 * - If the sum of agent fee amount and protocol fee amount is greater than the offer fee limit determined by the protocol
80 * - If the sum of agent fee amount and protocol fee amount is greater than fee limit set by seller
81 * - Royalty recipient is not on seller's allow list
82 * - Royalty percentage is less than the value decided by the admin
83 * - Total royalty percentage is more than max royalty percentage
84 *
85 * @param _offer - the fully populated struct with offer id set to 0x0 and voided set to false
86 * @param _offerDates - the fully populated offer dates struct
87 * @param _offerDurations - the fully populated offer durations struct
88 * @param _drParameters - the id of chosen dispute resolver (can be 0) and mutualizer address (0 for self-mutualization)
89 * @param _agentId - the id of agent
90 * @param _feeLimit - the maximum fee that seller is willing to pay per exchange (for static offers)
91 * @param _authenticate - whether to authenticate the caller. Use false when called from a handler that already authenticated the caller.
92 */
93function createOfferInternal(
94 Offer memory _offer,
95 OfferDates calldata _offerDates,
96 OfferDurations calldata _offerDurations,
97 BosonTypes.DRParameters calldata _drParameters,
98 uint256 _agentId,
99 uint256 _feeLimit,
100 bool _authenticate
101) internal returns (uint256 offerId) {
102 address sender = _msgSender();
103
104 if (_offer.creator == OfferCreator.Seller) {
105 if (_offer.buyerId != 0) revert InvalidBuyerOfferFields();
106
107 // Validate caller is seller assistant
108 if (_authenticate) {
109 (bool isAssistant, uint256 sellerId) = getSellerIdByAssistant(sender);
110 if (!isAssistant) {
111 revert NotAssistant();
112 }
113 _offer.sellerId = sellerId;
114 }
115 } else if (_offer.creator == OfferCreator.Buyer) {
116 if (
117 _offer.sellerId != 0 ||
118 _offer.collectionIndex != 0 ||
119 _offer.royaltyInfo.length != 1 ||
120 _offer.royaltyInfo[0].recipients.length != 0 ||
121 _offer.royaltyInfo[0].bps.length != 0 ||
122 _drParameters.mutualizerAddress != address(0) ||
123 _offer.quantityAvailable != 1 ||
124 _offer.priceType != PriceType.Static
125 ) {
126 revert InvalidBuyerOfferFields();
127 }
128 if (_authenticate) _offer.buyerId = getValidBuyer(payable(sender));
129 }

Recommendation:

We advise them to be properly documented in line with the rest of the codebase.

Alleviation (efd5d1a8f23c3bca7c25273ea4c912a367250119):

The referenced conditions have been properly documented in the function's revert cases, addressing this exhibit.