LF-14074 - Allow multiple aduits for contract version #1204
Conversation
|
Warning Rate limit exceeded@lifi-action-bot has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe audit verification workflow was updated to support multiple audits per contract version. The script now deduplicates audit IDs, ensures at least one audit exists, and validates required fields for each audit. Previous checks tied to a single audit, such as commit hash association and PR review approval, are now disabled. Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🤖 GitHub Action: Security Alerts Review 🔍✅ No unresolved security alerts! 🎉 |
Test Coverage ReportLine Coverage: 84.03% (2305 / 2743 lines) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/versionControlAndAuditCheck.yml (1)
508-517: Optional: Warn when multiple audits exist
To improve visibility, consider adding a warning if more than one audit is logged, for example:if [[ $AUDIT_COUNT -gt 1 ]]; then echo -e "\033[33mWarning: Found $AUDIT_COUNT audits for $FILENAME v$VERSION; using the first one ($AUDIT_ID).\033[0m" fiThis does not change logic but alerts maintainers to multiple audit entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/versionControlAndAuditCheck.yml(1 hunks)src/Libraries/LibAllowList.sol(1 hunks)
🔇 Additional comments (2)
src/Libraries/LibAllowList.sol (1)
9-9: Approve version bump
The NatSpec@custom:versiontag has been updated to1.0.3, correctly reflecting the new library release..github/workflows/versionControlAndAuditCheck.yml (1)
508-513: Allow multiple audits per version
The audit verification step now usesif [[ $AUDIT_COUNT -lt 1 ]]to enforce a minimum of one audit, removing the previous failure on multiple entries. This aligns with LF-14074’s requirement to permit multiple audits while still failing when none are found.
lifi-action-bot
changed the title
lifi-action-bot
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/versionControlAndAuditCheck.yml (3)
591-598: Remove redundant auditor handle existence check
The GitHub handle existence is already validated once; the second block (lines 619–626) duplicates the same check. Remove one to DRY up the script.Also applies to: 619-626
604-604: Clean up disabled single-audit comments
The commented-out single-audit enforcement block can be removed or moved to a known deprecation section to improve readability.
507-507: Remove trailing whitespace
Lines 507, 527, and 668 contain trailing spaces. Please trim to satisfy YAML lint and maintain consistency.Also applies to: 527-527, 668-668
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 507-507: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/versionControlAndAuditCheck.yml(5 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/versionControlAndAuditCheck.yml
[error] 507-507: trailing spaces
(trailing-spaces)
[error] 527-527: trailing spaces
(trailing-spaces)
[error] 668-668: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (5)
.github/workflows/versionControlAndAuditCheck.yml (5)
24-24: Updated audit comment clearly documents multi-audit support
The new comment accurately reflects that multiple audits per contract version are now allowed.
505-513: Duplicate-ID detection logic is sound
Usingsort | uniqto dedupe and comparing counts correctly flags duplicates. Considersort -ufor brevity, but the current approach is clear and effective.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 507-507: trailing spaces
(trailing-spaces)
517-521: Enforce at least one audit entry
The check for$AUDIT_COUNT -lt 1cleanly errors when no audits exist. This aligns with the requirement to mandate at least one audit.
524-529: Loop over each audit ID for validation
Iterating through all unique audit IDs and extracting entries is correct and supports multi-audit workflows.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 527-527: trailing spaces
(trailing-spaces)
669-671: Final success message is clear
The closing logs andAuditCompletedassignment follow the new multi-audit flow correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/versionControlAndAuditCheck.yml (4)
525-592: Comprehensive audit entry validation
The loop over each audit ID correctly extracts and validates all required fields (auditCompletedOn,auditedBy,auditorGitHandle,auditReportPath,auditCommitHash) with appropriate error handling.Consider centralizing repeated validation patterns into a helper function or leveraging a single
jqinvocation to check all required keys at once for improved maintainability.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 527-527: trailing spaces
(trailing-spaces)
591-599: Remove duplicate GitHub handle existence checks
The auditor GitHub handle is verified twice withgh api users/.... Consolidate these into a single check to eliminate redundancy.Also applies to: 617-626
604-668: Prune deprecated single-audit enforcement code
The large commented-out block related to single-audit checks is no longer needed and clutters the workflow. Remove or archive it to improve readability.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 668-668: trailing spaces
(trailing-spaces)
507-507: Remove trailing spaces
YAML lint reports trailing spaces on these lines. Please clean them up to adhere to formatting standards.Also applies to: 527-527, 668-668
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 507-507: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/versionControlAndAuditCheck.yml(5 hunks)audit/auditLog.json(1 hunks)src/Libraries/LibAllowList.sol(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- audit/auditLog.json
- src/Libraries/LibAllowList.sol
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/versionControlAndAuditCheck.yml
[error] 507-507: trailing spaces
(trailing-spaces)
[error] 527-527: trailing spaces
(trailing-spaces)
[error] 668-668: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: enforce-min-test-coverage
🔇 Additional comments (3)
.github/workflows/versionControlAndAuditCheck.yml (3)
24-24: Document support for multiple audits in known limitations
Updating the known limitations to explicitly state that multiple audits per contract version are allowed is accurate and aligns with the PR objective.
505-514: Duplicate audit detection logic is sound
Usingsort | uniqto dedupe audit IDs and comparing the counts correctly prevents duplicate entries in the audit log.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 507-507: trailing spaces
(trailing-spaces)
517-523: Enforce at least one audit per version
The check for$AUDIT_COUNT -lt 1clearly ensures that a minimum of one audit is present, satisfying the requirement.
lifi-action-bot
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.github/workflows/versionControlAndAuditCheck.yml (6)
22-22: Update outdated limitation commentThe comment states that multiple audits per contract version is a known limitation, but the PR changes explicitly add support for multiple audits. Please remove or update this comment to reflect the new behavior.
503-512: Deduplication logic is solidThe
sort | uniqapproach correctly deduplicates audit IDs and errors on duplicates as intended. Consider condensingsort | uniqintosort -ufor brevity, and remove the blank line at line 505 to eliminate trailing whitespace.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 505-505: trailing spaces
(trailing-spaces)
526-527: Extract audit entry with jqThis correctly retrieves the audit JSON. For stronger failure semantics, consider using
jq -eso missing entries cause a non-zero exit rather than checking for"null"manually.
598-604: Initial GitHub handle existence checkThe
gh api users/...call reliably verifies the auditor’s GitHub handle. However, this block duplicates the later validation (lines 616–625). Consolidate into one to avoid redundant API calls.
616-625: Remove redundant GitHub handle validationThis second
gh api users/...check revalidates the same handle. Please remove one of these blocks to streamline the flow.
505-505: Remove trailing whitespaceLines 505, 525, and 666 contain trailing spaces, triggering YAML lint errors. Please remove the extra spaces at the ends of these lines.
Also applies to: 525-525, 666-666
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 505-505: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/versionControlAndAuditCheck.yml(5 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/versionControlAndAuditCheck.yml
[error] 505-505: trailing spaces
(trailing-spaces)
[error] 525-525: trailing spaces
(trailing-spaces)
[error] 666-666: trailing spaces
(trailing-spaces)
🔇 Additional comments (9)
.github/workflows/versionControlAndAuditCheck.yml (9)
515-520: Ensure at least one audit checkThe block correctly enforces that at least one audit is present (
AUDIT_COUNT -lt 1). This aligns with the PR objective. Implementation looks good.
529-533: Audit entry null-check is validChecking for
nulland empty string guards against missing or malformed entries. Good defensive measure.
536-537: These are simple logging statements and do not warrant changes.
539-545: Extract log entry variablesPulling out
auditCompletedOn,auditedBy,auditorGitHandle,auditReportPath, andauditCommitHashis correct and straightforward.
547-554: Validate audit completion dateThe date presence check and error messaging are clear and match requirements.
558-564: Validate auditor company nameEnsures
auditedByis non-empty before proceeding. Implementation is correct.
567-573: Validate auditor Git handle presenceChecks that
auditorGitHandleis provided. Good safeguard.
577-583: Validate audit report file existenceThe file existence check for
auditReportPathis necessary. The error messages are clear.
586-592: Validate audit commit hash presenceEnsuring
auditCommitHashis present before further validation is correct.
lifi-action-bot
changed the title
lifi-action-bot
changed the title
Which Jira task belongs to this PR?
LF-14074
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)