feat(Safenet): Safenet feature branch #4810

germartinez · 2025-01-22T09:09:06Z

What it solves

This PR creates a feature flag for Safenet, to hide or display all the related components.

Resolves https://github.com/safe-global/safenet/issues/277

How to test it

Add NEXT_PUBLIC_IS_SAFENET_ENABLED to your .env file and try NEXT_PUBLIC_IS_SAFENET_ENABLED=true vs. NEXT_PUBLIC_IS_SAFENET_ENABLED=false. When set to true all Safenet features/components should be displaed, otherwise the Safenet features should be hidden.

Checklist

I've tested the branch on mobile 📱
I've documented how it affects the analytics (if at all) 📊
I've written a unit/e2e test for it (if applicable) 🧑‍💻

github-actions · 2025-01-22T09:09:19Z

Branch preview

✅ Deploy successful!

Website:
https://safenet_feature_branch--walletweb.review.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

Storybook:
https://safenet_feature_branch--walletweb.review.5afe.dev/storybook/

github-actions · 2025-01-22T09:16:27Z

Coverage report for `apps/web`

Caution

Test run failed

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	74.08% (-0.05% 🔻)	14861/20060
🔴	Branches	51.69% (-0.16% 🔻)	3557/6881
🔴	Functions	56.88% (-0.11% 🔻)	2101/3694
🟡	Lines	75.61% (-0.04% 🔻)	13487/17837

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🔴	`...` / safenet.ts	34.62% (-7.05% 🔻)	0% (-100% 🔻)	16.67%	33.33% (-3.51% 🔻)
🟢	`...` / index.tsx	100%	80% (-20% 🔻)	100%	100%
🟢	`...` / index.tsx	66.67% (-33.33% 🔻)	0% (-83.33% 🔻)	0% (-100% 🔻)	80% (-20% 🔻)
🔴	`...` / index.tsx	31.71% (-19.51% 🔻)	0% (-18.75% 🔻)	0% (-25% 🔻)	32.5% (-17.5% 🔻)
🟢	`...` / config.tsx	88.89% (+0.89% 🔼)	50% (-50% 🔻)	100%	100%
🟡	`...` / SafenetTxChecks.tsx	61.11% (-1.39% 🔻)	0%	0%	64.71% (-1.96% 🔻)
🟢	`...` / useIsSafenetEnabled.ts	91.67% (-8.33% 🔻)	100%	100%	90.91% (-9.09% 🔻)
🟢	`...` / useLoadBalances.ts	81.67%	65% (-8.68% 🔻)	58.33%	83.33%
🟡	`...` / index.tsx	75% (-1.19% 🔻)	0%	0%	76.74% (-1.3% 🔻)

Test suite run failed

Failed tests: 2/1793. Failed suites: 1/242.

  ● ConfirmationView › should display a confirmation screen for a SETTINGS_CHANGE transaction

    expect(received).toMatchSnapshot()

    Snapshot name: `ConfirmationView should display a confirmation screen for a SETTINGS_CHANGE transaction 1`

    - Snapshot  -  8
    + Received  + 12

    @@ -279,18 +279,22 @@
                              data-testid="tx-data-row"
                            >
                              <div
                                class="MuiBox-root css-axw7ok"
                              >
    -                           <img
    -                             alt="ETH"
    -                             class="image"
    -                             height="26"
    -                             loading="lazy"
    -                             referrerpolicy="no-referrer"
    -                             src="/images/common/token-placeholder.svg"
    -                           />
    +                           <div
    +                             class="container"
    +                           >
    +                             <img
    +                               alt="ETH"
    +                               class="image"
    +                               height="26"
    +                               loading="lazy"
    +                               referrerpolicy="no-referrer"
    +                               src="/images/common/token-placeholder.svg"
    +                             />
    +                           </div>
                                <p
                                  class="MuiTypography-root MuiTypography-body1 css-w5uidf-MuiTypography-root"
                                >
                                  ETH
                                </p>

      116 |     )
      117 |
    > 118 |     expect(container).toMatchSnapshot()
          |                       ^
      119 |   })
      120 |
      121 |   it("should display a confirmation with method call when the transaction type is not found in the ConfirmationView's mapper", () => {

      at Object.toMatchSnapshot (src/components/tx/confirmation-views/ConfirmationView.test.tsx:118:23)

  ● ConfirmationView › should display a confirmation with method call when the transaction type is not found in the ConfirmationView's mapper

    expect(received).toMatchSnapshot()

    Snapshot name: `ConfirmationView should display a confirmation with method call when the transaction type is not found in the ConfirmationView's mapper 1`

    - Snapshot  -  8
    + Received  + 12

    @@ -168,18 +168,22 @@
                              data-testid="tx-data-row"
                            >
                              <div
                                class="MuiBox-root css-axw7ok"
                              >
    -                           <img
    -                             alt="ETH"
    -                             class="image"
    -                             height="26"
    -                             loading="lazy"
    -                             referrerpolicy="no-referrer"
    -                             src="/images/common/token-placeholder.svg"
    -                           />
    +                           <div
    +                             class="container"
    +                           >
    +                             <img
    +                               alt="ETH"
    +                               class="image"
    +                               height="26"
    +                               loading="lazy"
    +                               referrerpolicy="no-referrer"
    +                               src="/images/common/token-placeholder.svg"
    +                             />
    +                           </div>
                                <p
                                  class="MuiTypography-root MuiTypography-body1 css-w5uidf-MuiTypography-root"
                                >
                                  ETH
                                </p>

      131 |     )
      132 |
    > 133 |     expect(container).toMatchSnapshot()
          |                       ^
      134 |   })
      135 | })
      136 |

      at Object.toMatchSnapshot (src/components/tx/confirmation-views/ConfirmationView.test.tsx:133:23)

Report generated by 🧪jest coverage report action from 2d927d6

github-actions · 2025-01-22T10:26:29Z

📦 Next.js Bundle Analysis for @safe-global/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page	Size (compressed)
`global`	`1.05 MB` (🟡 +63.01 KB)

Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

New Page Added

The following page was added to the bundle from the code in this PR:

Page	Size (compressed)	First Load
`/settings/safenet`	`218.48 KB`	1.26 MB

Thirty-one Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page	Size (compressed)	First Load
`/`	`508 B` (🟢 -2 B)	1.05 MB
`/address-book`	`23.21 KB` (🟡 +142 B)	1.07 MB
`/apps`	`35.51 KB` (🟡 +1.78 KB)	1.09 MB
`/apps/custom`	`33.57 KB` (🟡 +1.78 KB)	1.08 MB
`/apps/open`	`55.68 KB` (🟡 +2.07 KB)	1.1 MB
`/balances`	`29.89 KB` (🟡 +190 B)	1.08 MB
`/balances/nfts`	`9.52 KB` (🟢 -23 B)	1.06 MB
`/bridge`	`2.56 KB` (🟡 +2 B)	1.05 MB
`/cookie`	`8.77 KB` (🟡 +1 B)	1.06 MB
`/home`	`61.19 KB` (🟡 +1.93 KB)	1.11 MB
`/new-safe/advanced-create`	`26.38 KB` (🟢 -70 B)	1.08 MB
`/new-safe/create`	`25.52 KB` (🟢 -71 B)	1.08 MB
`/privacy`	`14.57 KB` (🟡 +2 B)	1.06 MB
`/settings/appearance`	`2.25 KB` (🟡 +2 B)	1.05 MB
`/settings/environment-variables`	`3.27 KB` (🟢 -1 B)	1.05 MB
`/settings/modules`	`4.06 KB` (🟡 +1 B)	1.05 MB
`/settings/notifications`	`6.33 KB` (🟢 -14.99 KB)	1.06 MB
`/settings/safe-apps`	`20.35 KB` (🟡 +2.08 KB)	1.07 MB
`/settings/security`	`2.34 KB` (🟡 +1 B)	1.05 MB
`/settings/setup`	`30.83 KB` (🟡 +94 B)	1.08 MB
`/share/safe-app`	`7.56 KB` (🟢 -5 B)	1.06 MB
`/stake`	`618 B` (🟢 -1 B)	1.05 MB
`/swap`	`761 B` (🟡 +1 B)	1.05 MB
`/terms`	`12.93 KB` (🟡 +1 B)	1.06 MB
`/transactions`	`99.58 KB` (🟡 +3 KB)	1.15 MB
`/transactions/history`	`99.54 KB` (🟡 +3 KB)	1.15 MB
`/transactions/messages`	`60.37 KB` (🟡 +2.07 KB)	1.11 MB
`/transactions/msg`	`56.62 KB` (🟡 +2.07 KB)	1.11 MB
`/transactions/queue`	`49.47 KB` (🟡 +2.08 KB)	1.1 MB
`/transactions/tx`	`48.83 KB` (🟡 +2.07 KB)	1.1 MB
`/welcome/accounts`	`409 B` (🟡 +2 B)	1.05 MB

Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

schmanu · 2025-01-22T10:48:05Z

apps/web/src/components/common/TokenIcon/index.tsx

@@ -35,7 +36,7 @@ const TokenIcon = ({
        referrerPolicy="no-referrer"
        loading="lazy"
      />
-      {safenet && (
+      {IS_SAFENET_ENABLED && safenet && (


I think we should additionally have a new Feature flag on the config service and use this .env flag to overwrite it temporarily such that we do not enable it on staging yet.

This way we can later just toggle the feature by adjusting the config and do not need to make a new release

schmanu · 2025-01-22T10:55:46Z

apps/web/src/components/transactions/TxDetails/Summary/index.tsx

-          </GradientBoxSafenet>
-        </TxDataRow>
-      </Box>
+      {IS_SAFENET_ENABLED && (


Can we not use the SafenetTxChecks component directly? It looks very similar.

apps/web/src/components/tx/SignOrExecuteForm/SafenetTxChecks.tsx

schmanu · 2025-01-22T10:59:41Z

apps/web/src/hooks/useIsSafenetEnabled.ts

@@ -6,6 +7,10 @@ const useIsSafenetEnabled = () => {
  const { safe } = useSafeInfo()
  const { data: safenetConfig } = useGetSafenetConfigQuery()

+  if (!IS_SAFENET_ENABLED) {


As mentioned above lets introduce a new feature flag change this to:

Suggested change

if (!IS_SAFENET_ENABLED) {

const hasFeature = useHasFeature(FEATURES.SAFENET)

const isSafenetEnabled = IS_SAFENET_ENABLED || hasFeature

if (!isSafenetEnabled) {

schmanu · 2025-01-22T11:00:16Z

apps/web/src/hooks/useIsSafenetEnabled.ts

@@ -6,6 +7,10 @@ const useIsSafenetEnabled = () => {
  const { safe } = useSafeInfo()
  const { data: safenetConfig } = useGetSafenetConfigQuery()


I think we should pass the skipToken here in case safenet is not enabled such that RTK query skips the request.

katspaugh · 2025-01-22T11:12:04Z

Please also make sure the entire feature is encapsulated in one folder and is exported from a single, lazy-loaded index.ts module depending on the feature flag as per https://www.notion.so/safe-global/Feature-toggles-970c96d18e544fd1a01a362f0535146b

germartinez added 2 commits January 21, 2025 16:38

Add IS_SAFENET_ENABLED flag

efdba74

Hide Safenet components behind feature flag

1f55e9a

germartinez requested a review from schmanu January 22, 2025 09:09

germartinez added 2 commits January 22, 2025 10:06

Code formatting

ae6dfc8

Fix ESLint errors

2d927d6

schmanu requested changes Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Safenet): Safenet feature branch #4810

feat(Safenet): Safenet feature branch #4810

germartinez commented Jan 22, 2025

github-actions bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025

schmanu Jan 22, 2025

schmanu Jan 22, 2025

schmanu Jan 22, 2025

schmanu Jan 22, 2025

katspaugh commented Jan 22, 2025 •

edited

Loading

-  if (!IS_SAFENET_ENABLED) {
+const hasFeature = useHasFeature(FEATURES.SAFENET)
+const isSafenetEnabled = IS_SAFENET_ENABLED || hasFeature
+  if (!isSafenetEnabled) {

		@@ -6,6 +7,10 @@ const useIsSafenetEnabled = () => {
		const { safe } = useSafeInfo()
		const { data: safenetConfig } = useGetSafenetConfigQuery()

feat(Safenet): Safenet feature branch #4810

Are you sure you want to change the base?

feat(Safenet): Safenet feature branch #4810

Conversation

germartinez commented Jan 22, 2025

What it solves

How to test it

Checklist

github-actions bot commented Jan 22, 2025 • edited Loading

Branch preview

github-actions bot commented Jan 22, 2025 • edited Loading

Coverage report for apps/web

Test suite run failed

github-actions bot commented Jan 22, 2025

📦 Next.js Bundle Analysis for @safe-global/web

⚠️ Global Bundle Size Increased

New Page Added

Thirty-one Pages Changed Size

schmanu Jan 22, 2025

Choose a reason for hiding this comment

schmanu Jan 22, 2025

Choose a reason for hiding this comment

schmanu Jan 22, 2025

Choose a reason for hiding this comment

schmanu Jan 22, 2025

Choose a reason for hiding this comment

katspaugh commented Jan 22, 2025 • edited Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

Coverage report for `apps/web`

katspaugh commented Jan 22, 2025 •

edited

Loading