Skip to content
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.

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

Update and clarify documentation comments #5206

Merged
merged 16 commits into from
Sep 23, 2024
3 changes: 1 addition & 2 deletions contracts/finance/VestingWalletCliff.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ abstract contract VestingWalletCliff is VestingWallet {
error InvalidCliffDuration(uint64 cliffSeconds, uint64 durationSeconds);

/**
* @dev Sets the sender as the initial owner, the beneficiary as the pending owner, the start timestamp, the
* vesting duration and the duration of the cliff of the vesting wallet.
* @dev Set the start timestamp of the vesting wallet cliff.
*/
constructor(uint64 cliffSeconds) {
if (cliffSeconds > duration()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ abstract contract GovernorCountingFractional is Governor {
uint256 againstVotes = 0;
uint256 forVotes = 0;
uint256 abstainVotes = 0;
uint256 usedWeight;
uint256 usedWeight = 0;

// For clarity of event indexing, fractional voting must be clearly advertised in the "support" field.
//
Expand Down
2 changes: 1 addition & 1 deletion contracts/interfaces/IERC1363Spender.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
pragma solidity ^0.8.20;

/**
* @title ERC1363Spender
* @title IERC1363Spender
* @dev Interface for any contract that wants to support `approveAndCall`
* from ERC-1363 token contracts.
*/
Expand Down
8 changes: 4 additions & 4 deletions contracts/utils/cryptography/P256.sol
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ library P256 {

/**
* @dev Checks if (x, y) are valid coordinates of a point on the curve.
* In particular this function checks that x <= P and y <= P.
* In particular this function checks that x < P and y < P.
*/
function isValidPublicKey(bytes32 x, bytes32 y) internal pure returns (bool result) {
assembly ("memory-safe") {
Expand Down Expand Up @@ -239,7 +239,7 @@ library P256 {
}

/**
* @dev Compute P·u1 + Q·u2 using the precomputed points for P and Q (see {_preComputeJacobianPoints}).
* @dev Compute G·u1 + Q·u2 using the precomputed points for G and Q (see {_preComputeJacobianPoints}).
cairoeth marked this conversation as resolved.
Show resolved Hide resolved
*
* Uses Strauss Shamir trick for EC multiplication
* https://stackoverflow.com/questions/50993471/ec-scalar-multiplication-with-strauss-shamir-method
Expand Down Expand Up @@ -299,10 +299,10 @@ library P256 {
points[0x09] = _jAddPoint(points[0x01], points[0x08]); // 1,2 (p+2g)
points[0x0a] = _jAddPoint(points[0x02], points[0x08]); // 2,2 (2p+2g)
points[0x0b] = _jAddPoint(points[0x03], points[0x08]); // 3,2 (3p+2g)
points[0x0c] = _jAddPoint(points[0x04], points[0x08]); // 0,3 (g+2g)
points[0x0c] = _jAddPoint(points[0x04], points[0x08]); // 0,3 (3g)
cairoeth marked this conversation as resolved.
Show resolved Hide resolved
points[0x0d] = _jAddPoint(points[0x01], points[0x0c]); // 1,3 (p+3g)
points[0x0e] = _jAddPoint(points[0x02], points[0x0c]); // 2,3 (2p+3g)
points[0x0f] = _jAddPoint(points[0x03], points[0x0C]); // 3,3 (3p+3g)
points[0x0f] = _jAddPoint(points[0x03], points[0x0c]); // 3,3 (3p+3g)
}

function _jAddPoint(JPoint memory p1, JPoint memory p2) private pure returns (JPoint memory) {
Expand Down
6 changes: 3 additions & 3 deletions contracts/utils/cryptography/RSA.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ library RSA {
* method described in https://datatracker.ietf.org/doc/html/rfc8017#section-8.2.2[section 8.2.2 of RFC8017].
*
* IMPORTANT: Although this function allows for it, using n of length 1024 bits is considered unsafe.
* Consider using at least 2048 bits.
* Consider using at least 2048 bits. Additionally, this function only supports SHA256 as the hash function.
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of removing this note and rename the function to pkcs1Sha256 just as its bytes counterpart. That was the original naming iiurc but I proposed renaming it while ignoring the AlgorithmIdentifier is included during signature creation

*
* WARNING: PKCS#1 v1.5 allows for replayability given the message may contain arbitrary optional parameters in the
* DigestInfo. Consider using an onchain nonce or unique identifier to include in the message to prevent replay attacks.
Expand Down Expand Up @@ -94,13 +94,13 @@ library RSA {
// it should be at 32 (digest) + 2 bytes from the end. To those 34 bytes, we add the
// OID (9 bytes) and its length (2 bytes) to get the position of the DigestInfo sequence,
// which is expected to have a length of 0x31 when the NULL param is present or 0x2f if not.
if (bytes1(_unsafeReadBytes32(buffer, length - 50)) == 0x31) {
if (bytes1(_unsafeReadBytes32(buffer, length - 0x32)) == 0x31) {
offset = 0x34;
// 00 (1 byte) | SEQUENCE length (0x31) = 3031 (2 bytes) | SEQUENCE length (0x0d) = 300d (2 bytes) | OBJECT_IDENTIFIER length (0x09) = 0609 (2 bytes)
// SHA256 OID = 608648016503040201 (9 bytes) | NULL = 0500 (2 bytes) (explicit) | OCTET_STRING length (0x20) = 0420 (2 bytes)
params = 0x003031300d060960864801650304020105000420000000000000000000000000;
mask = 0xffffffffffffffffffffffffffffffffffffffff000000000000000000000000; // (20 bytes)
} else if (bytes1(_unsafeReadBytes32(buffer, length - 48)) == 0x2F) {
} else if (bytes1(_unsafeReadBytes32(buffer, length - 0x30)) == 0x2F) {
offset = 0x32;
// 00 (1 byte) | SEQUENCE length (0x2f) = 302f (2 bytes) | SEQUENCE length (0x0b) = 300b (2 bytes) | OBJECT_IDENTIFIER length (0x09) = 0609 (2 bytes)
// SHA256 OID = 608648016503040201 (9 bytes) | NULL = <implicit> | OCTET_STRING length (0x20) = 0420 (2 bytes)
Expand Down
5 changes: 3 additions & 2 deletions contracts/utils/structs/CircularBuffer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ library CircularBuffer {
* directly. Use the functions provided below instead. Modifying the struct manually may violate assumptions and
* lead to unexpected behavior.
*
* The last item is at data[(index - 1) % data.length] and the last item is at data[index % data.length]. This
* range can wrap around.
* In a full buffer:
* - The most recently pushed item (last) is at data[(index - 1) % data.length]
* - The oldest item (first) is at data[index % data.length]
*/
struct Bytes32CircularBuffer {
uint256 _count;
Expand Down
2 changes: 0 additions & 2 deletions contracts/utils/structs/MerkleTree.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import {Panic} from "../Panic.sol";
* NOTE: Building trees using non-commutative hashing functions (i.e. `H(a, b) != H(b, a)`) is supported. However,
* proving the inclusion of a leaf in such trees is not possible with the {MerkleProof} library since it only supports
* _commutative_ hashing functions.
*
* _Available since v5.1._
cairoeth marked this conversation as resolved.
Show resolved Hide resolved
*/
library MerkleTree {
/**
Expand Down