feat: integrate ubeswap savings widget#623
Open
Oluwatomilola wants to merge 2 commits intoGoodDollar:masterfrom
Open
feat: integrate ubeswap savings widget#623Oluwatomilola wants to merge 2 commits intoGoodDollar:masterfrom
Oluwatomilola wants to merge 2 commits intoGoodDollar:masterfrom
Conversation
Oluwatomilola
requested a deployment
to
external
GitHub Actions
Waiting
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 6 issues, and left some high level feedback:
- Consider removing
temp_widget.ts, which defines a secondgooddollar-savings-widgetimplementation that is not wired into the app and largely duplicatesSavingsWidget.ts, to avoid confusion and accidental divergence. src/savings-sdk-source.tsappears to export from./viem-sdkand./wagmi-sdk, but the actual implementation lives underutils/savings-sdk/viem-sdk.tsand there is nowagmi-sdkin this diff; either wire this file up correctly or delete it if it’s a leftover.- In
SavingsWidget, theisLoadingflag is never set totrue/falsearound the async calls inrefreshData/loadStats/loadUserStats, so the UI’sLoading...states will never show; either remove the conditional loading text or track the loading state properly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider removing `temp_widget.ts`, which defines a second `gooddollar-savings-widget` implementation that is not wired into the app and largely duplicates `SavingsWidget.ts`, to avoid confusion and accidental divergence.
- `src/savings-sdk-source.ts` appears to export from `./viem-sdk` and `./wagmi-sdk`, but the actual implementation lives under `utils/savings-sdk/viem-sdk.ts` and there is no `wagmi-sdk` in this diff; either wire this file up correctly or delete it if it’s a leftover.
- In `SavingsWidget`, the `isLoading` flag is never set to `true/false` around the async calls in `refreshData`/`loadStats`/`loadUserStats`, so the UI’s `Loading...` states will never show; either remove the conditional loading text or track the loading state properly.
## Individual Comments
### Comment 1
<location> `src/components/Savings/SavingsWidget.ts:566-575` </location>
<code_context>
+ private validateInput(force = false) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard `parseEther` against malformed but regex-accepted inputs (e.g. trailing decimal point) to avoid runtime errors.
The current regex allows strings like `"1."` or `"."`, which `parseFloat` accepts but cause `parseEther` to throw, leading to uncaught exceptions. Either tighten the regex to reject trailing/standalone dots or wrap `parseEther` in try/catch and set `inputError` on failure to make the input handling safe.
</issue_to_address>
### Comment 2
<location> `src/components/Savings/SavingsWidget.ts:278-287` </location>
<code_context>
+ isLoading: { type: Boolean, state: true },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider actually driving `isLoading` to reflect fetch state instead of leaving it permanently `false`.
`isLoading` is never set to `true`, so the UI will never show the loading state for balances, rewards, or APR. Consider toggling `isLoading` around `refreshData` / `loadStats` / `loadUserStats` so users see loading feedback during network requests.
</issue_to_address>
### Comment 3
<location> `src/savings-sdk-source.ts:1-3` </location>
<code_context>
+export * from './viem-sdk'
+export { useGooddollarSavings } from './wagmi-sdk'
+export type { GlobalStats, UserStats } from './viem-sdk'
</code_context>
<issue_to_address>
**issue (bug_risk):** The re-export paths here look inconsistent with the actual file location under `utils/savings-sdk`.
Since `GooddollarSavingsSDK` lives in `src/utils/savings-sdk/viem-sdk.ts`, these root-level exports will look for `src/viem-sdk.ts`, which doesn’t exist. If this is intended as a public entrypoint, the exports should target `'./utils/savings-sdk/viem-sdk'` (and similarly adjust the path for `wagmi-sdk`) to avoid build-time failures.
</issue_to_address>
### Comment 4
<location> `temp_widget.ts:7-8` </location>
<code_context>
+import { celo } from 'viem/chains';
+import { GooddollarSavingsSDK } from '@goodsdks/savings-sdk';
+
+@customElement('gooddollar-savings-widget')
+export class GooddollarSavingsWidget extends LitElement {
+ static styles = css`
+ :host {
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid registering the same custom element name twice across `temp_widget.ts` and `SavingsWidget.ts`.
This module also defines `gooddollar-savings-widget`, but `src/components/Savings/SavingsWidget.ts` already registers that tag via `customElements.define`. If both modules run, the second registration will throw a `DOMException`. If this file is only experimental, consider excluding it from the build or using a different tag name.
</issue_to_address>
### Comment 5
<location> `src/components/Savings/SavingsWidget.ts:15` </location>
<code_context>
+import { celo } from 'viem/chains'
+import { GooddollarSavingsSDK } from '../../utils/savings-sdk'
+
+export class SavingsWidget extends LitElement {
+ static get styles() {
+ return css`
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the shared savings business logic (SDK setup, state, actions, and validation) into a reusable controller used by both this widget and `temp_widget.ts` to keep the components focused on UI only.
You can keep this implementation but reduce complexity (and the duplication with `temp_widget.ts`) by extracting the “savings logic” into a shared controller and using it from both widgets.
### 1. Extract SDK + state + actions into a controller
Move all non‑UI concerns out of the component:
- web3 wiring (`walletClient`, `publicClient`, `sdk`, `userAddress`)
- data loading (`refreshData`, `loadStats`, `loadUserStats`, `resetUserStats`)
- staking flows (`handleStake`, `handleUnstake`, `handleClaim`)
- formatting / validation (`formatBigInt`, `toEtherNumber`, `formatPercent`, `validateInput`)
For example, create `savings-controller.ts`:
```ts
// savings-controller.ts
import {
createWalletClient,
createPublicClient,
custom,
PublicClient,
WalletClient,
http,
formatEther,
parseEther,
} from 'viem'
import { celo } from 'viem/chains'
import { GooddollarSavingsSDK } from '../../utils/savings-sdk'
export type SavingsState = {
walletBalance: bigint
currentStake: bigint
unclaimedRewards: bigint
totalStaked: bigint
userWeeklyRewards: bigint
annualAPR: number
}
export class SavingsController {
private walletClient: WalletClient | null = null
private publicClient: PublicClient | null = null
private sdk: GooddollarSavingsSDK | null = null
private userAddress: string | null = null
state: SavingsState = {
walletBalance: 0n,
currentStake: 0n,
unclaimedRewards: 0n,
totalStaked: 0n,
userWeeklyRewards: 0n,
annualAPR: 0,
}
constructor(private web3Provider?: any) {}
async refreshData() {
if (!this.publicClient) {
this.publicClient = createPublicClient({
chain: celo,
transport: http(),
}) as unknown as PublicClient
}
if (this.web3Provider && this.web3Provider.isConnected) {
this.walletClient = createWalletClient({
chain: celo,
transport: custom(this.web3Provider),
})
this.sdk = new GooddollarSavingsSDK(this.publicClient!, this.walletClient)
await this.loadStats()
const accounts = await this.web3Provider.request({ method: 'eth_accounts' })
if (accounts.length > 0) {
this.userAddress = accounts[0]
await this.loadUserStats()
} else {
this.resetUserStats()
}
} else {
this.sdk = new GooddollarSavingsSDK(this.publicClient!)
await this.loadStats()
}
}
private async loadStats() {
if (!this.sdk) return
const globalStats = await this.sdk.getGlobalStats()
this.state.totalStaked = globalStats.totalStaked
this.state.annualAPR = globalStats.annualAPR
}
private async loadUserStats() {
if (!this.sdk || !this.userAddress) return
const userStats = await this.sdk.getUserStats()
this.state.walletBalance = userStats.walletBalance
this.state.currentStake = userStats.currentStake
this.state.unclaimedRewards = userStats.unclaimedRewards
this.state.userWeeklyRewards = userStats.userWeeklyRewards
}
private resetUserStats() {
this.state.walletBalance = 0n
this.state.currentStake = 0n
this.state.unclaimedRewards = 0n
this.state.userWeeklyRewards = 0n
}
formatBigInt(num: bigint) {
if (!num) return '0'
return Intl.NumberFormat().format(this.toEtherNumber(num))
}
toEtherNumber(num: bigint) {
return Number(formatEther(num))
}
formatPercent(num: number) {
return `${num.toFixed(2)}%`
}
validateInput(
inputAmount: string,
activeTab: 'stake' | 'unstake',
): { error: string; amountWei?: bigint } {
if (!inputAmount || inputAmount.trim() === '') {
return { error: '' }
}
const validInputRegex = /^[0-9]*\.?[0-9]*$/
if (!validInputRegex.test(inputAmount)) {
return { error: 'Invalid value' }
}
const numValue = parseFloat(inputAmount)
if (isNaN(numValue) || numValue <= 0) {
return { error: 'Please enter a valid amount' }
}
const amountWei = parseEther(inputAmount)
if (activeTab === 'stake' && amountWei > this.state.walletBalance) {
return { error: 'Insufficient balance' }
}
if (activeTab === 'unstake' && amountWei > this.state.currentStake) {
return { error: 'Max amount exceeded' }
}
return { error: '', amountWei }
}
async stake(amountWei: bigint) {
if (!this.sdk || !this.userAddress) throw new Error('Wallet not connected')
const receipt = await this.sdk.stake(amountWei)
await this.refreshData()
return receipt
}
async unstake(amountWei: bigint) {
if (!this.sdk || !this.userAddress) throw new Error('Wallet not connected')
const receipt = await this.sdk.unstake(amountWei)
await this.refreshData()
return receipt
}
async claim() {
if (!this.sdk || !this.userAddress) throw new Error('Wallet not connected')
const receipt = await this.sdk.claimReward()
await this.refreshData()
return receipt
}
}
```
### 2. Use the controller in this widget (and `temp_widget.ts`)
The widget becomes mostly wiring + template, and `temp_widget.ts` can reuse the same controller:
```ts
// savings-widget.ts
import { SavingsController } from './savings-controller'
export class SavingsWidget extends LitElement {
controller?: SavingsController
static get properties() {
return {
web3Provider: { type: Object },
// ...
inputAmount: { type: String, state: true },
inputError: { type: String, state: true },
}
}
constructor() {
super()
this.inputAmount = '0.0'
this.inputError = ''
}
updated(changedProps: Map<string, any>) {
if (changedProps.has('web3Provider')) {
this.controller = new SavingsController(this.web3Provider)
void this.refreshData()
}
}
private async refreshData() {
if (!this.controller) return
this.isLoading = true
await this.controller.refreshData()
const s = this.controller.state
this.walletBalance = s.walletBalance
this.currentStake = s.currentStake
this.unclaimedRewards = s.unclaimedRewards
this.totalStaked = s.totalStaked
this.userWeeklyRewards = s.userWeeklyRewards
this.annualAPR = s.annualAPR
this.isLoading = false
}
private handleInputChange = (e: Event) => {
this.inputAmount = (e.target as HTMLInputElement).value
this.validateInput()
}
private validateInput(force = false) {
if (!this.controller) return
const { error } = this.controller.validateInput(
this.inputAmount,
this.activeTab as 'stake' | 'unstake',
)
this.inputError = force && !this.inputAmount ? 'Please enter a valid amount' : error
}
private handleStake = async () => {
if (!this.controller) return
const { error, amountWei } = this.controller.validateInput(
this.inputAmount,
'stake',
)
if (error || !amountWei) {
this.inputError = error
return
}
try {
this.txLoading = true
this.transactionError = ''
await this.controller.stake(amountWei)
this.inputAmount = '0.0'
this.inputError = ''
} catch (e: any) {
this.transactionError = e.message || 'Staking failed'
} finally {
this.txLoading = false
}
}
// handleUnstake / handleClaim can mirror handleStake but call controller.unstake / controller.claim
}
```
`temp_widget.ts` can then instantiate `SavingsController` in the same way, but keep its current decorators / class name / SDK import path differences. This removes the duplicated business logic and makes future changes to staking/claiming flows, validation, and formatting single‑sourced while preserving all current behavior.
</issue_to_address>
### Comment 6
<location> `temp_widget.ts:8` </location>
<code_context>
+import { GooddollarSavingsSDK } from '@goodsdks/savings-sdk';
+
+@customElement('gooddollar-savings-widget')
+export class GooddollarSavingsWidget extends LitElement {
+ static styles = css`
+ :host {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the shared savings widget business logic into a reusable controller/module so both this widget and the existing one become thin UI wrappers instead of duplicating code.
The main complexity here is maintaining two near-identical implementations of the same widget. You can keep this new `gooddollar-savings-widget` while eliminating duplication by extracting the shared logic into a reusable module and turning both components into thin wrappers.
For example, pull the SDK wiring, state shape, and handlers into a class or composable function:
```ts
// src/components/Savings/sharedSavingsLogic.ts
import { GooddollarSavingsSDK } from '@goodsdks/savings-sdk';
import { createWalletClient, createPublicClient, custom, http, formatEther, parseEther } from 'viem';
import { celo } from 'viem/chains';
export interface SavingsState {
activeTab: 'stake' | 'unstake';
inputAmount: string;
walletBalance: bigint;
currentStake: bigint;
unclaimedRewards: bigint;
totalStaked: bigint;
userWeeklyRewards: bigint;
annualAPR: number;
isLoading: boolean;
txLoading: boolean;
isClaiming: boolean;
inputError: string;
transactionError: string;
}
export class SavingsController {
private publicClient = createPublicClient({ chain: celo, transport: http() });
private walletClient: any = null;
private sdk: GooddollarSavingsSDK | null = null;
private userAddress: string | null = null;
state: SavingsState = {
activeTab: 'stake',
inputAmount: '0.0',
walletBalance: 0n,
currentStake: 0n,
unclaimedRewards: 0n,
totalStaked: 0n,
userWeeklyRewards: 0n,
annualAPR: 0,
isLoading: false,
txLoading: false,
isClaiming: false,
inputError: '',
transactionError: '',
};
constructor(private getWeb3Provider: () => any) {}
async refreshData() {
const web3Provider = this.getWeb3Provider();
if (web3Provider && web3Provider.isConnected) {
this.walletClient = createWalletClient({ chain: celo, transport: custom(web3Provider) });
this.sdk = new GooddollarSavingsSDK(this.publicClient, this.walletClient);
await this.loadStats();
const accounts = await web3Provider.request({ method: 'eth_accounts' });
this.userAddress = accounts[0] ?? null;
this.userAddress ? await this.loadUserStats() : this.resetUserStats();
} else {
this.sdk = new GooddollarSavingsSDK(this.publicClient);
await this.loadStats();
}
}
// ...move loadStats, loadUserStats, resetUserStats, format helpers,
// validateInput, handleStake/Unstake/Claim into methods here...
}
```
Then in this Lit component, bind to the controller instead of duplicating all logic:
```ts
// temp_widget.ts (or gooddollar-savings-widget.ts)
import { SavingsController } from './sharedSavingsLogic';
@customElement('gooddollar-savings-widget')
export class GooddollarSavingsWidget extends LitElement {
@property({ type: Object }) web3Provider: any = null;
@property({ type: Function }) connectWallet?: () => void;
private controller = new SavingsController(() => this.web3Provider);
// expose controller.state to the template:
get state() {
return this.controller.state;
}
connectedCallback() {
super.connectedCallback();
this.interval = setInterval(() => this.controller.refreshData(), 30_000);
this.controller.refreshData();
}
// template stays almost the same but uses `this.state.*`
render() {
const { activeTab, inputAmount, walletBalance, currentStake } = this.state;
// ...
}
}
```
The existing `SavingsWidget.ts` can then use the same `SavingsController` (with or without decorators), so only the Lit wiring and markup differences live in each file, and all business logic (SDK interaction, validation, refresh, error handling) is defined once.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Oluwatomilola
requested a deployment
to
external
GitHub Actions
Waiting
Contributor
|
@Oluwatomilola Hey. |
3 tasks
Author
|
Apologies for the mixup. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR integrates the GoodDollar Savings Widget into GoodDapp as a first-class page.
Changes include:
Added a new Savings menu item to the sidebar.
Created a new /savings page using the same layout structure as Bridge/Swap/Claim.
Embedded the GooddollarSavingsWidget via a dedicated wrapper component.
Added contextual FAQs for the Savings feature.
Introduced a small SDK abstraction layer to support wallet providers (wagmi/viem).
This implements the requirements described in issue #619.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: