From d406c7b71b309d689aa1114b75f53db87bcb438a Mon Sep 17 00:00:00 2001 From: mfw78 Date: Thu, 30 Jul 2026 09:48:25 +0000 Subject: [PATCH] style: use plain comments for in-body implementation notes solc only recognises NatSpec on declarations, so a `@dev` tag inside a function body is inert: it documents nothing and no tooling reads it. Demote those to ordinary `//` and `/* */` comments and drop the tag, so that a doc comment in the tree always means NatSpec. This is the companion to the NatSpec standardisation earlier in the stack, which deliberately left in-body comments alone. It is separable from the rest of the train and can be dropped without affecting anything below it. --- script/deploy_AnvilStack.s.sol | 4 +-- src/BaseConditionalOrder.sol | 2 +- src/types/GoodAfterTime.sol | 6 ++-- src/types/PerpetualStableSwap.sol | 2 +- src/types/StopLoss.sol | 14 ++++---- src/types/TradeAboveThreshold.sol | 2 +- src/types/twap/TWAP.sol | 12 +++---- src/types/twap/libraries/TWAPOrderMathLib.sol | 32 ++++++++++--------- test/helpers/CoWProtocol.t.sol | 8 ++--- 9 files changed, 43 insertions(+), 39 deletions(-) diff --git a/script/deploy_AnvilStack.s.sol b/script/deploy_AnvilStack.s.sol index 478535fb..21975d27 100644 --- a/script/deploy_AnvilStack.s.sol +++ b/script/deploy_AnvilStack.s.sol @@ -97,10 +97,10 @@ contract DeployAnvilStack is Script { GPv2AllowListAuthentication allowList = new GPv2AllowListAuthentication(); allowList.initializeManager(all); - /// @dev the settlement contract is the main entry point for the CoW Protocol + // the settlement contract is the main entry point for the CoW Protocol settlement = new GPv2Settlement(allowList, vault); - /// @dev the relayer is the account authorized to spend the user's tokens + // the relayer is the account authorized to spend the user's tokens relayer = address(settlement.vaultRelayer()); // authorize the solver diff --git a/src/BaseConditionalOrder.sol b/src/BaseConditionalOrder.sol index 7f09e76c..601ab9bd 100644 --- a/src/BaseConditionalOrder.sol +++ b/src/BaseConditionalOrder.sol @@ -44,7 +44,7 @@ abstract contract BaseConditionalOrder is IConditionalOrderGenerator, IOrderMani ) external view override { GPv2Order.Data memory generatedOrder = generateOrder(owner, sender, ctx, staticInput, offchainInput); - /// @dev Verify that the *generated* order is valid and matches the payload. + // Verify that the *generated* order is valid and matches the payload. require( _hash == GPv2Order.hash(generatedOrder, domainSeparator), IConditionalOrder.OrderNotValid(InvalidHash.selector) diff --git a/src/types/GoodAfterTime.sol b/src/types/GoodAfterTime.sol index ee54abc7..9c6cf479 100644 --- a/src/types/GoodAfterTime.sol +++ b/src/types/GoodAfterTime.sol @@ -82,8 +82,10 @@ contract GoodAfterTime is OrderDescriptor { // Decode the payload into the good after time parameters. Data memory data = abi.decode(staticInput, (Data)); - /// @dev A fill-or-kill order with a zero sell amount never trips the - /// settlement replay guard; reject it at the source. + /* + * A fill-or-kill order with a zero sell amount never trips the + * settlement replay guard; reject it at the source. + */ require(data.sellAmount > 0, IConditionalOrder.OrderNotValid(ZeroAmount.selector)); // Don't allow the order to be placed before it becomes valid. diff --git a/src/types/PerpetualStableSwap.sol b/src/types/PerpetualStableSwap.sol index 5e6999f7..1d1fdcab 100644 --- a/src/types/PerpetualStableSwap.sol +++ b/src/types/PerpetualStableSwap.sol @@ -61,7 +61,7 @@ contract PerpetualStableSwap is OrderDescriptor { override returns (GPv2Order.Data memory order) { - /// @dev Decode the payload into the perpetual stable swap parameters. + // Decode the payload into the perpetual stable swap parameters. PerpetualStableSwap.Data memory data = abi.decode(staticInput, (Data)); // Always sell whatever of the two tokens we have more of diff --git a/src/types/StopLoss.sol b/src/types/StopLoss.sol index cbb23a1e..ff10c9a0 100644 --- a/src/types/StopLoss.sol +++ b/src/types/StopLoss.sol @@ -92,12 +92,14 @@ contract StopLoss is OrderDescriptor { { Data memory data = abi.decode(staticInput, (Data)); - /// @dev A fill-or-kill order with a zero amount never trips the settlement - /// replay guard; reject it at the source. + /* + * A fill-or-kill order with a zero amount never trips the settlement + * replay guard; reject it at the source. + */ require(data.sellAmount > 0 && data.buyAmount > 0, IConditionalOrder.OrderNotValid(ZeroAmount.selector)); // scope variables to avoid stack too deep error { - /// @dev Guard against expired orders + // Guard against expired orders if (data.validTo < block.timestamp) { revert IConditionalOrder.OrderNotValid(OrderExpired.selector); } @@ -105,10 +107,10 @@ contract StopLoss is OrderDescriptor { (, int256 basePrice,, uint256 sellUpdatedAt,) = data.sellTokenPriceOracle.latestRoundData(); (, int256 quotePrice,, uint256 buyUpdatedAt,) = data.buyTokenPriceOracle.latestRoundData(); - /// @dev Guard against invalid price data + // Guard against invalid price data require(basePrice > 0 && quotePrice > 0, IConditionalOrder.OrderNotValid(OracleInvalidPrice.selector)); - /// @dev Guard against stale data at a user-specified interval. The maxTimeSinceLastOracleUpdate should at least exceed the both oracles' update intervals. + // Guard against stale data at a user-specified interval. The maxTimeSinceLastOracleUpdate should at least exceed the both oracles' update intervals. require( sellUpdatedAt >= block.timestamp - data.maxTimeSinceLastOracleUpdate && buyUpdatedAt >= block.timestamp - data.maxTimeSinceLastOracleUpdate, @@ -120,7 +122,7 @@ contract StopLoss is OrderDescriptor { basePrice = Utils.scalePrice(basePrice, data.sellTokenPriceOracle.decimals(), 18); quotePrice = Utils.scalePrice(quotePrice, data.buyTokenPriceOracle.decimals(), 18); - /// @dev Scale the strike price to 18 decimals. + // Scale the strike price to 18 decimals. require( basePrice * SCALING_FACTOR / quotePrice <= data.strike, IConditionalOrderGenerator.PollTryNextBlock(StrikeNotReached.selector) diff --git a/src/types/TradeAboveThreshold.sol b/src/types/TradeAboveThreshold.sol index 4cb8e364..4d1e4681 100644 --- a/src/types/TradeAboveThreshold.sol +++ b/src/types/TradeAboveThreshold.sol @@ -41,7 +41,7 @@ contract TradeAboveThreshold is BaseConditionalOrder { override returns (GPv2Order.Data memory order) { - /// @dev Decode the payload into the trade above threshold parameters. + // Decode the payload into the trade above threshold parameters. TradeAboveThreshold.Data memory data = abi.decode(staticInput, (Data)); uint256 balance = data.sellToken.balanceOf(owner); diff --git a/src/types/twap/TWAP.sol b/src/types/twap/TWAP.sol index a071785a..dac04148 100644 --- a/src/types/twap/TWAP.sol +++ b/src/types/twap/TWAP.sol @@ -60,8 +60,8 @@ contract TWAP is OrderDescriptor { override returns (GPv2Order.Data memory order) { - /** - * @dev Decode the payload into a TWAP bundle and get the order. `orderFor` will revert if + /* + * Decode the payload into a TWAP bundle and get the order. `orderFor` will revert if * there is no current valid order. * NOTE: This will return an order even if the part of the TWAP bundle that is currently * valid is filled. This is safe as CoW Protocol ensures that each `orderUid` is only @@ -71,8 +71,8 @@ contract TWAP is OrderDescriptor { order = TWAPOrder.orderFor(twap); - /** - * @dev If outside the current part's span, this is a scheduling gap, not a + /* + * If outside the current part's span, this is a scheduling gap, not a * permanent failure: signal when the next part starts, or terminate if * there is no next part. */ @@ -215,9 +215,7 @@ contract TWAP is OrderDescriptor { { twap = abi.decode(staticInput, (TWAPOrder.Data)); - /** - * @dev If `twap.t0` is set to 0, then get the start time from the context. - */ + // If `twap.t0` is set to 0, then get the start time from the context. if (twap.t0 == 0) { twap.t0 = uint256(composableCow.cabinet(owner, ctx)); } diff --git a/src/types/twap/libraries/TWAPOrderMathLib.sol b/src/types/twap/libraries/TWAPOrderMathLib.sol index 90b3601b..91fe0413 100644 --- a/src/types/twap/libraries/TWAPOrderMathLib.sol +++ b/src/types/twap/libraries/TWAPOrderMathLib.sol @@ -32,8 +32,8 @@ library TWAPOrderMathLib { view returns (uint256 validTo) { - /** - * @dev Use `assert` to check for invalid inputs as these should be caught by the + /* + * Use `assert` to check for invalid inputs as these should be caught by the * conditional order validation logic in `dispatch` before calling this function. * This is to save on gas deployment costs vs using `require` statements. */ @@ -42,15 +42,17 @@ library TWAPOrderMathLib { assert(span <= frequency); unchecked { - /// @dev Order is not yet valid before the start (order commences at `t0`); - /// a monitoring service should try again at the start time. + /* + * Order is not yet valid before the start (order commences at `t0`); + * a monitoring service should try again at the start time. + */ require( startTime <= block.timestamp, IConditionalOrderGenerator.PollTryAtTimestamp(startTime, BeforeTwapStart.selector) ); - /** - * @dev Order is expired after the last part (`n` parts, running at `t` time length). + /* + * Order is expired after the last part (`n` parts, running at `t` time length). * * Multiplication overflow: `numParts` is bounded by `type(uint32).max` and `frequency` is bounded by * `365 days` which is smaller than `type(uint32).max` so the product of `numParts * frequency` is @@ -63,8 +65,8 @@ library TWAPOrderMathLib { IConditionalOrder.OrderNotValid(AfterTwapFinish.selector) ); - /** - * @dev We use integer division to get the part number as we want to round down to the nearest part. + /* + * We use integer division to get the part number as we want to round down to the nearest part. * * Subtraction underflow: `startTime` is asserted to be less than `block.timestamp` so the difference * of `block.timestamp - startTime` shall always be positive. @@ -73,9 +75,9 @@ library TWAPOrderMathLib { uint256 part = (block.timestamp - startTime) / frequency; // calculate the `validTo` timestamp (inclusive as per `GPv2Order`) if (span == 0) { - /** - * @dev If the span is zero, then the order is valid for the entire part. - * We can safely add `part + 1` to `part` as we know that `part` is less than `numParts`. + /* + * If the span is zero, then the order is valid for the entire part. + * We can safely add `part + 1` to `part` as we know that `part` is less than `numParts`. * * Addition overflow: `part` is bounded by `numParts` which is bounded by `type(uint32).max` so * the sum of `part + 1` is ≈ 2³². @@ -90,8 +92,8 @@ library TWAPOrderMathLib { return startTime + ((part + 1) * frequency) - 1; } - /** - * @dev If the span is non-zero, then the order is valid for the span of the part. + /* + * If the span is non-zero, then the order is valid for the span of the part. * * Multiplication overflow: `part` is bounded by `numParts` which is bounded by `type(uint32).max` with * `frequency` bounded by `365 days` which is smaller than `type(uint32).max` so the product of @@ -105,8 +107,8 @@ library TWAPOrderMathLib { */ validTo = startTime + (part * frequency) + span - 1; - /** - * @dev Order is not valid if not within nominated span. This doesn't need to be asserted as it is + /* + * Order is not valid if not within nominated span. This doesn't need to be asserted as it is * checked during settlement in `GPv2Settlement.settle`. */ } diff --git a/test/helpers/CoWProtocol.t.sol b/test/helpers/CoWProtocol.t.sol index ed62acd8..6e6bea8b 100644 --- a/test/helpers/CoWProtocol.t.sol +++ b/test/helpers/CoWProtocol.t.sol @@ -45,9 +45,9 @@ abstract contract CoWProtocol is Test, Tokens { */ function setUp() public virtual { // cowprotocol test accounts - /// @dev the admin account is used to deploy the vault and allow list manager + // the admin account is used to deploy the vault and allow list manager admin = TestAccountLib.createTestAccount("admin"); - /// @dev the solver account simulates a solver in the allow list + // the solver account simulates a solver in the allow list solver = TestAccountLib.createTestAccount("solver"); vault = IVault(makeAddr("fakeVault")); @@ -56,10 +56,10 @@ abstract contract CoWProtocol is Test, Tokens { GPv2AllowListAuthentication allowList = new GPv2AllowListAuthentication(); allowList.initializeManager(admin.addr); - /// @dev the settlement contract is the main entry point for the CoW Protocol + // the settlement contract is the main entry point for the CoW Protocol settlement = new GPv2Settlement(allowList, vault); - /// @dev the relayer is the account authorized to spend the user's tokens + // the relayer is the account authorized to spend the user's tokens relayer = address(settlement.vaultRelayer()); // authorize the solver