') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix: solidity coding style by NicolasRampoldi · Pull Request #843 · yetanotherco/aligned_layer · GitHub
Skip to content

fix: solidity coding style - #843

Merged
NicolasRampoldi merged 31 commits into
mainfrom
835-fix-solidity-coding-style-guideline-violations
Aug 26, 2024
Merged

fix: solidity coding style#843
NicolasRampoldi merged 31 commits into
mainfrom
835-fix-solidity-coding-style-guideline-violations

Conversation

@NicolasRampoldi

@NicolasRampoldiNicolasRampoldi commented Aug 22, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Description

  • Adds solhint linter to the CI. To run solhint locally, go to contracts, run npm install and then npm run lint:sol. The CI will run the linter but won't stop the user from pushing to main, it just lists the warnings.
  • Fixes most linter warnings, the ones left don't seem relevant.
  • The three rules added were taken from https://github.com/lambdaclass/era-contracts/blob/main/.solhint.json and it made sense to add them to this repo too.
  • Add make lint_contracts to makefile

To Test

  • Try deploying and test normal flow.
  • Make some errors on purpose to see if the refactoring of requires to custom errors works properly.

@NicolasRampoldiNicolasRampoldi linked an issue Aug 22, 2024 that may be closed by this pull request
fix: Solidity Coding Style Guideline Violations #835
Closed
@NicolasRampoldiNicolasRampoldi self-assigned this Aug 22, 2024
@github-actions

github-actionsBot commented Aug 22, 2024•
edited
Loading

Copy link
Copy Markdown

Changes to gas cost

Generated at commit: bc78cc93f7fa31d0ab2ff3a1fbc79cadd9a20962, compared to commit: 19e76e0f07a383afcba000953afed50d8ae10964

🧾 Summary (10% most significant diffs)

ContractMethodAvg (+/-)%
AlignedLayerServiceManagercreateNewTask-27 ✅-0.05%
Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,359,036 (-46,214)createNewTask51,138 (0)0.00%51,319 (-27)-0.05%51,234 (-12)-0.02%51,936 (0)0.00%256 (0)

@NicolasRampoldiNicolasRampoldi linked an issue Aug 22, 2024 that may be closed by this pull request
fix: Unused Imports #837
Closed
@NicolasRampoldi
Merge branch 'main' into 835-fix-solidity-coding-style-guideline-viol…
7d79014
…ations
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/AlignedLayerServiceManager.sol
@NicolasRampoldi
fix: merge conflicts
a49ed0f
uri-99
uri-99 reviewed Aug 22, 2024
View reviewed changes

@uri-99uri-99 left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

.

uri-99
uri-99 suggested changes Aug 22, 2024
View reviewed changes

@uri-99uri-99 left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Everything seems to be working fine, and read it over for quite some time, but we should talk about this PR because it is pretty big and potentially dangerous.

I used to like a lot the require statements, but custom errors are pretty powerful, mainly because of accepting parameters. More info on this: https://soliditylang.org/blog/2021/04/21/custom-errors/

It looks like it should be compatible with an upgrade, with no need of changing the GAP, but this should be tested more in detail just in case.

Also, should errors be declared in the Interface? While we're at it, Events could also be declared in the Interface.

Also I believe this PR should require at least 3 or even 4 approves.

Finally, a make target to run solhint should come in pretty handy.

@NicolasRampoldi
feat: add lint to makefile
332b080
@NicolasRampoldi
Merge branch 'main' into 835-fix-solidity-coding-style-guideline-viol…
d780e6d
…ations
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/AlignedLayerServiceManagerStorage.sol
@NicolasRampoldi
chore: merge conflicts
0677327
@NicolasRampoldi
refactor: move events and errors to interface
3bb64cd
taturosati
taturosati suggested changes Aug 23, 2024
View reviewed changes
Comment threadcontracts/src/core/BatcherPaymentService.sol Outdated
Comment threadcontracts/src/core/BatcherPaymentService.sol Outdated
Comment threadcontracts/src/core/BatcherPaymentService.sol Outdated
taturosati
taturosati approved these changes Aug 23, 2024
View reviewed changes

@taturosatitaturosati left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code lgtm, worked in my machine

@NicolasRampoldi
fix: linter
1cb2866
@NicolasRampoldi
Merge branch 'main' into 835-fix-solidity-coding-style-guideline-viol…
f1829b2
…ations
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: anvil state
f830819
@glpecileglpecile mentioned this pull request Aug 23, 2024
Merged
@NicolasRampoldi
Merge branch 'main' into 835-fix-solidity-coding-style-guideline-viol…
4392f54
…ations
# Conflicts:
#	contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
#	contracts/src/core/AlignedLayerServiceManager.sol
#	contracts/src/core/BatcherPaymentService.sol
@NicolasRampoldi
chore: merge conflicts
f537707
@NicolasRampoldi
NicolasRampoldi merged commit c26f1ee into mainAug 26, 2024
@NicolasRampoldi
NicolasRampoldi deleted the 835-fix-solidity-coding-style-guideline-violations branch August 26, 2024 20:10
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@uri-99uri-99uri-99 approved these changes
@taturosatitaturosatitaturosati approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@NicolasRampoldiNicolasRampoldi

Labels

auditdue-diligencetrail-of-bits

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: Unused Imports fix: Solidity Coding Style Guideline Violations

4 participants

@NicolasRampoldi@uri-99@taturosati@entropidelic