Show current position in Ledger Live dApp

[ioay]

Closes: #111

---

Show current position in Ledger Live dApp

---

What was done:

• created hook to get shares & estimated bitcoin balance from acre-sdk on initialization dApp and keeping it in redux store.
• added serializableCheck to redux middleware (https://redux-toolkit.js.org/api/serializabilityMiddleware)

---

Additional info:

• Conversion to USD is out of the scope of this task.

[netlify]

✅ Deploy Preview for acre-dapp-testnet ready!

Name	Link
🔨 Latest commit	6b291f1
🔍 Latest deploy log	https://app.netlify.com/sites/acre-dapp-testnet/deploys/661505ff54d3660008b89f1f
😎 Deploy Preview	https://deploy-preview-302--acre-dapp-testnet.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[r-czajkowski]

I'm not convinced that this is the best place to keep it. sharesBalance balance is actually stBTC balance and from the developers perspective it could be misleading. I'm wondering if we shouldn't do something like we did in Threshold dashboard - create slice for tokens see:

• https://github.com/threshold-network/token-dashboard/blob/main/src/store/tokens/tokenSlice.ts#L33
• https://github.com/threshold-network/token-dashboard/blob/main/src/hooks/useTokenBalance.ts

[ioay]

As we agreed, let's do it in a separate task: #315, because of touching some not-related parts of the code.

[kkosiorowska]

I also agree. I think we should also give this a higher priority to organize the redux structure.

[r-czajkowski]

Why did we switch it off?

[ioay]

Because without this we get an error in console, when fetching BTC price from coingecko:

useFetchBTCPriceUSD.ts:10 A non-serializable value was detected in the state, in the path: `btc.estimatedBtcBalance`. Value: 0n 
Take a look at the reducer(s) handling this action type: btc/fetchBTCPriceUSD/fulfilled.

#302 (comment)

[r-czajkowski]

That's weird because bigint is a primitive type. Looks like it is a bug in redux-toolkit see https://redux-toolkit.js.org/api/serializabilityMiddleware#isplain. But I think we can override it.

import {
  configureStore,
  createSerializableStateInvariantMiddleware,
  isPlain as reduxToolkitIsPlain,
} from "@reduxjs/toolkit"
import { middleware } from "./middleware"
import { reducer } from "./reducer"

const isPlain = (value: unknown) => {
  const type = typeof value
  return reduxToolkitIsPlain(value) || type === "bigint"
}

const serializableMiddleware = createSerializableStateInvariantMiddleware({
  isSerializable: isPlain,
})

export const store = configureStore({
  reducer,
  middleware: (getDefaultMiddleware) =>
    getDefaultMiddleware(middleware).concat(serializableMiddleware),
  devTools: !import.meta.env.PROD,
})

export type RootState = ReturnType<typeof store.getState>
export type AppDispatch = typeof store.dispatch

[ioay]

That's weird because bigint is a primitive type

Indeed, bigint is non-seriazable, and it's why we get this error:
A non-serializable value was detected in the state.

Redux doc says:

/**

• The function to check if a value is considered serializable. This
• function is applied recursively to every value contained in the
• state. Defaults to isPlain().
  */
  isSerializable?: (value: any) => boolean

I'm not sure if overriding isPlain function is a good idea, sounds like a workaround, and in the future, we will have to extend this function to all primitive types.
In my opinion, disabling serializableCheck is more convenient and I don't see any risk in turning it off at this point.
If there is such a thing in the future, we might consider overriding isPlain, what do you think?

[Shadowfiend]

Note that including non-serializable data in the store will break things like redux devtools. For reference in the future, here is how we set this up in Taho, where we included remote devtools explicitly (vs having it built in via browser extension, since we were already inside an extension):

https://github.com/tahowallet/extension/blob/4a93eba515c6a63fd5c4aa4d44a05775355b5792/background/main.ts#L204-L220

https://github.com/tahowallet/extension/blob/4a93eba515c6a63fd5c4aa4d44a05775355b5792/background/main.ts#L246-L289

[r-czajkowski]

Ah I thought, the bigint is serializable but BigInt is not but they are actually the same type sorry. I found this in the docs, maybe it will be helpful https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#use_within_json

[kkosiorowska]

I also agree. I think we should also give this a higher priority to organize the redux structure.

[kkosiorowska]

Why do we need it?

[ioay]

To meet the type of restriction: GetDefaultEnhancersOptions

[kkosiorowska]

It seems that this parameter is optional. In that case, can we just skip it? I feel that this creates a bit of confusion.

[ioay]

yes, I've checked this before, but we still get an error without it:

Argument of type '{ devToolsEnhancer: StoreEnhancer; } | { devToolsEnhancer?: undefined; }' is not assignable to parameter of type 'GetDefaultEnhancersOptions | undefined'.
Type '{ devToolsEnhancer: StoreEnhancer; }' has no properties in common with type 'GetDefaultEnhancersOptions'.

[kkosiorowska]

To clarify. Why do we need this package?

[ioay]

@redux-devtools/extension is required to configure devToolsEnhancer

[kkosiorowska]

But why do we need to configure devToolsEnhancer? I wonder why we didn't configure it in devTools? 🤔 I would like to understand this better because I remember that in Taho dApp did not install an external package for this.

[ioay]

It was commented on here: #302 (comment)
But of course, there is no problem with doing the same in a different way that you proposed and was used in the Taho dApp.

cf86aa5

[kkosiorowska]

I was just curious why you used an external library.

[kkosiorowska]

I left only minor comments/questions.

I noticed that PR has been merged earlier. The description includes the following information:

Additional info:
the amount of stBTC will be updated when #302 is merged

Let's not miss it and make sure to correctly display the amount of stBTC. We can do it in this PR or the next but let's not lose it.

[ioay]

I left only minor comments/questions.

I noticed that PR has been merged earlier. The description includes the following information:

Additional info:
the amount of stBTC will be updated when #302 is merged

Let's not miss it and make sure to correctly display the amount of stBTC. We can do it in this PR or the next but let's not lose it.

Yep, I added shares balance to LiquidStakingTokenPopover in this PR : d71a13b

[kkosiorowska]

Overall, it looks good. Let's fix the build and merge it. 🚢