Skip to content

feat: Add smart accounts reference generation script and docs pages #716

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

Open
wants to merge 65 commits into
base: main
Choose a base branch
from

Conversation

louis-md
Copy link
Contributor

@louis-md louis-md commented Mar 11, 2025

This PR adds a smart-account-reference section, generated with the latest Safe smart account versions, using pnpm generate-smart-account-reference.

@louis-md louis-md requested a review from valle-xyz March 11, 2025 17:12
Copy link

github-actions bot commented Mar 11, 2025

Branch preview

✅ Deployed successfully in branch deployment:

https://modules_reference--docs.review.5afe.dev

Copy link

github-actions bot commented Mar 11, 2025

Overall readability score: 36.34 (🔴 -0.08)

File Readability
ChangedGuard.mdx 100 (-)
ChangedFallbackHandler.mdx 100 (-)
ApproveHash.mdx 100 (-)
AddedOwner.mdx 100 (-)
safe-contracts-deployment.mdx 53.11 (🔴 -0.22)
add-or-edit-chain.mdx 85.83 (🟢 +0)
smart-account-signatures.md 37.39 (🟢 +0)
smart-account-modules.mdx 35.24 (🟢 +0)
smart-account-migration.mdx 57.63 (🟢 +0.16)
safe-proxy-factory.mdx 65.48 (🔴 -1.42)
multi-chain-deployment.mdx 58.46 (🟢 +0)
VirtualCallout.mdx 12.68 (-)
ReentrancyCallout.mdx 12.68 (-)
PreApprovedCallout.mdx 12.68 (-)
LegacyCallout.mdx 12.68 (-)
IrreversibilityCallout.mdx 12.68 (-)
View detailed metrics

🟢 - Shows an increase in readability
🔴 - Shows a decrease in readability

File Readability FRE GF ARI CLI DCRS
ChangedGuard.mdx 100 100 6 6 6 4.9
  - - - - - -
ChangedFallbackHandler.mdx 100 100 6 6 6 4.9
  - - - - - -
ApproveHash.mdx 100 100 6 6 6 4.9
  - - - - - -
AddedOwner.mdx 100 100 6 6 6 4.9
  - - - - - -
safe-contracts-deployment.mdx 53.11 40.35 8.32 14.3 15.19 7.4
  🔴 -0.22 🟢 +0 🔴 -0.13 🟢 +0 🟢 +0 🟢 +0
add-or-edit-chain.mdx 85.83 70.19 6.49 6.6 8.79 6.02
  🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0
smart-account-signatures.md 37.39 40.58 12.64 18.7 17.29 7.09
  🟢 +0 🔴 -0.11 🔴 -0.01 🟢 +0 🟢 +0 🟢 +0.01
smart-account-modules.mdx 35.24 30.77 13.51 14.1 15.42 9.69
  🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0
smart-account-migration.mdx 57.63 49.52 9.07 12.1 13.96 7.6
  🟢 +0.16 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0.05
safe-proxy-factory.mdx 65.48 49.52 9.34 9.6 11.12 7.39
  🔴 -1.42 🔴 -2.84 🔴 -0.66 🔴 -1.1 🔴 -0.06 🟢 +0.58
multi-chain-deployment.mdx 58.46 49.01 10.88 10.8 12.29 7.67
  🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0
VirtualCallout.mdx 12.68 0 11.6 22 19 11
  - - - - - -
ReentrancyCallout.mdx 12.68 0 11.6 22 19 11
  - - - - - -
PreApprovedCallout.mdx 12.68 0 11.6 22 19 11
  - - - - - -
LegacyCallout.mdx 12.68 0 11.6 22 19 11
  - - - - - -
IrreversibilityCallout.mdx 12.68 0 11.6 22 19 11
  - - - - - -

Averages:

  Readability FRE GF ARI CLI DCRS
Average 36.34 32.15 13.13 16.65 15.11 8.6
  🔴 -0.08 🔴 -0.17 🔴 -0.05 🔴 -0.19 🟢 +0.04 🟢 +0.08
View metric targets
Metric Range Ideal score
Flesch Reading Ease 100 (very easy read) to 0 (extremely difficult read) 60
Gunning Fog 6 (very easy read) to 17 (extremely difficult read) 8 or less
Auto. Read. Index 6 (very easy read) to 14 (extremely difficult read) 8 or less
Coleman Liau Index 6 (very easy read) to 17 (extremely difficult read) 8 or less
Dale-Chall Readability 4.9 (very easy read) to 9.9 (extremely difficult read) 6.9 or less

@louis-md louis-md marked this pull request as ready for review March 11, 2025 17:29
@akshay-ap
Copy link
Member

When v1.5.0 is released, there will be a new setModuleGuard function. How will the generation script handle this?

@louis-md
Copy link
Contributor Author

louis-md commented Apr 7, 2025

Thanks @nlordell for the thorough review! You raised valid points. I'm working on fixing the examples and sample values that you mentioned.

For the NatSpec content, could it be considered that we create a new npm package release (like what was done for the reference module v0.3.0-1) which will contain the fixed NatSpec comments? So these mistakes are also fixed in the developers' IDEs? If yes we can then take all NatSpec-related feedback and discussion from this PR directly in the contracts repo, and park this PR until all these fixes are released 🙏

@nlordell
Copy link
Collaborator

nlordell commented Apr 8, 2025

For the NatSpec content, could it be considered that we create a new npm package release (like what was done for the reference module v0.3.0-1) which will contain the fixed NatSpec comments?

I think the NatSpec comment issues were for the Safe contracts and not the modules (although the modules may also contain issues, but I hope not - in any case I didn't see any on my first pass).

I do not want to release a new version of contracts with corrected NatSpec documentation as this would lead changes to contract bytecode and break deterministic deployment.

However, for future deployments we are taking NatSpec documentation more seriously and including proof reading it a part of our pre-release checklist. In particular, we are nearing the release of the Safe v1.5.0 contracts and will proof read the NatSpec documentation before cutting the release and starting the deployments.

@louis-md louis-md force-pushed the modules-reference branch from 59e8ccb to 17b2022 Compare April 14, 2025 09:47
@louis-md louis-md changed the title Modules reference feat: Add smart accounts reference generation script and docs pages Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants