feat: add pool membership and reward claiming functionality

[EmekaManuel]

Description

Previously, members could only be added manually, and there was no easy way for members to claim their rewards. This PR adds join pool and claim reward functionality.

About

#308

Motivation

Previously, members could only be added manually, and there was no easy way for members to claim their rewards. This PR adds join pool and claim reward functionality.

Features

• Join Pool Button - Shows on collective pages for open UBI pools (onlyMembers = false). Includes transaction confirmation flow.
• Claim Reward Button - Shows eligible reward amount for pool members. Displays even if pool has no funds (shows G$ 0.0000).
• Claim Timer - Shows countdown until next claim is available after a member has already claimed.
• Auto-refresh - Membership status updates automatically after joining.

Implementation

New Hooks

• usePoolMembership - Checks if user is a member
• usePoolRewards - Gets eligible amount, claim status, and next claim time
• useJoinPool - Handles join pool transaction
• useClaimReward - Handles claim reward transaction
• usePoolOpenStatus - Checks if pool is open for new members

New Components

• JoinPoolButton - Join pool button with transaction flow
• ClaimRewardButton - Claim reward button showing eligible amount
• ClaimTimer - Countdown timer component

Updated

• ViewCollective - Added join/claim buttons based on pool state
• BaseModal - Fixed missing alt props on Image components
• Subgraph queries - Added ubiLimits and membersValidator fields

Notes

• Only available for UBI pools (DirectPayments pools handle membership through NFT claims)
• Membership status checked via contract calls for real-time accuracy
• Transaction flows use existing BaseModal pattern for consistency

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• In ViewCollective, relying on window.location.reload() in the JoinPoolButton onSuccess handlers is brittle (and may not work in native environments); prefer updating local/subgraph state via the existing refetch hooks or state setters instead of forcing a full reload.
• ClaimTimer accepts claimPeriodDays and poolName in its props but never uses them, which is confusing; either incorporate these values into the UI copy or remove the unused props.
• usePoolRewards computes and returns isPoolOpen based on ubiSettings but none of the call sites use this value (they use usePoolOpenStatus instead), so consider removing the isPoolOpen logic from usePoolRewards to avoid duplication and dead code.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In ViewCollective, relying on window.location.reload() in the JoinPoolButton onSuccess handlers is brittle (and may not work in native environments); prefer updating local/subgraph state via the existing refetch hooks or state setters instead of forcing a full reload.
- ClaimTimer accepts claimPeriodDays and poolName in its props but never uses them, which is confusing; either incorporate these values into the UI copy or remove the unused props.
- usePoolRewards computes and returns isPoolOpen based on ubiSettings but none of the call sites use this value (they use usePoolOpenStatus instead), so consider removing the isPoolOpen logic from usePoolRewards to avoid duplication and dead code.

## Individual Comments

### Comment 1
<location> `packages/app/src/components/ViewCollective.tsx:277-287` </location>
<code_context>
                 <View style={styles.collectiveDonateBox}>
+                  {/* Join Pool Button - show if pool is open and user is not a member (only for UBI pools) */}
+                  {isPoolOpen && !isMember && address && pooltype === 'UBI' && (
+                    <JoinPoolButton
+                      poolAddress={poolAddress as `0x${string}`}
+                      poolType={pooltype}
+                      poolName={ipfs?.name}
+                      onSuccess={() => {
+                        // Refetch membership status
+                        window.location.reload();
+                      }}
+                    />
</code_context>

<issue_to_address>
**suggestion:** Avoid using window.location.reload and rely on state/query updates instead.

Using window.location.reload() is heavy-handed, breaks the SPA flow, and may be a no-op or problematic in native environments. Since you already have membership/refetch hooks (e.g., usePoolMembership, subgraph hooks), please trigger those refetches or update local state on success instead so the UI updates without a full page reload.

```suggestion
                  {isPoolOpen && !isMember && address && pooltype === 'UBI' && (
                    <JoinPoolButton
                      poolAddress={poolAddress as `0x${string}`}
                      poolType={pooltype}
                      poolName={ipfs?.name}
                      onSuccess={async () => {
                        // Refetch membership status without reloading the page
                        await refetchMembership();
                      }}
                    />
                  )}
```
</issue_to_address>

### Comment 2
<location> `packages/app/src/components/ClaimTimer.tsx:5-11` </location>
<code_context>
+import { View, Text, VStack } from 'native-base';
+import { formatTime } from '../lib/formatTime';
+
+interface ClaimTimerProps {
+  nextClaimTime: number;
+  claimPeriodDays: number;
+  poolName?: string;
+}
+
+export const ClaimTimer: React.FC<ClaimTimerProps> = ({ nextClaimTime }) => {
+  const [timeRemaining, setTimeRemaining] = useState<number>(0);
+
</code_context>

<issue_to_address>
**suggestion:** ClaimTimer props include fields that aren’t actually used.

ClaimTimerProps includes claimPeriodDays and poolName, but the component only consumes nextClaimTime. Either remove the unused props (and update call sites) or use them in the UI if you plan to show pool-specific or period-based messaging, so the props type accurately reflects what the component needs.

Suggested implementation:

```typescript
interface ClaimTimerProps {
  nextClaimTime: number;
}

export const ClaimTimer: React.FC<ClaimTimerProps> = ({ nextClaimTime }) => {

```

You will also need to:
1. Update all call sites of ClaimTimer to stop passing claimPeriodDays and poolName props, for example:
   - Change: `<ClaimTimer nextClaimTime={...} claimPeriodDays={...} poolName="..." />`
   - To: `<ClaimTimer nextClaimTime={...} />`
2. If you still need pool-specific or period-based messaging elsewhere, consider either:
   - Handling it in the parent component that renders ClaimTimer, or
   - Re-introducing those props later but actually consuming them in the UI (e.g., adding text like “Next claim for {poolName} in {claimPeriodDays} days”).
</issue_to_address>

### Comment 3
<location> `packages/app/src/hooks/usePoolRewards.ts:43` </location>
<code_context>
+  },
+] as const;
+
+export function usePoolRewards(poolAddress: `0x${string}` | undefined, poolType: string | undefined) {
+  const { address, chain } = useAccount();
+
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring the hook to factor shared enabled conditions and separate reward state from pool-open logic to make it easier to read and maintain correctly.

You can shave off a fair bit of cognitive load here with a couple of small refactors, without changing behavior.

1) Factor shared enabled logic

Right now the `enabled` condition is duplicated with slight variations, which makes it harder to scan and easier to get wrong when updated.

You can pull this into a couple of booleans:

```ts
export function usePoolRewards(poolAddress: `0x${string}` | undefined, poolType: string | undefined) {
  const { address, chain } = useAccount();

  const isUBIPool = poolType === 'UBI';
  const hasPoolAddress = !!poolAddress;
  const hasAddress = !!address;

  const enabledUBI = hasPoolAddress && isUBIPool;
  const enabledUBIWithAddress = enabledUBI && hasAddress;

  const { data: ubiSettings } = useReadContract({
    chainId: chain?.id,
    address: poolAddress,
    abi: UBI_POOL_ABI,
    functionName: 'ubiSettings',
    query: { enabled: enabledUBI },
  });

  const { data: eligibleAmount } = useReadContract({
    chainId: chain?.id,
    address: poolAddress,
    abi: UBI_POOL_ABI,
    functionName: 'checkEntitlement',
    args: [address as `0x${string}`],
    query: { enabled: enabledUBIWithAddress },
  });

  const { data: hasClaimed } = useReadContract({
    chainId: chain?.id,
    address: poolAddress,
    abi: UBI_POOL_ABI,
    functionName: 'hasClaimed',
    args: [address as `0x${string}`],
    query: { enabled: enabledUBIWithAddress },
  });

  const { data: nextClaimTime } = useReadContract({
    chainId: chain?.id,
    address: poolAddress,
    abi: UBI_POOL_ABI,
    functionName: 'nextClaimTime',
    query: { enabled: enabledUBI },
  });

  // ...
}
```

This keeps the “when do we read?” rules in one place and makes future changes safer.

2) Reduce responsibility / ambiguity of pool-open logic

The hook is named `usePoolRewards` but also exposes an `isPoolOpen` flag based on `ubiSettings.onlyMembers`, while there’s already a separate `usePoolOpenStatus` hook using different criteria. That increases the chance of divergent behavior and misuse.

A lighter-weight alternative that preserves behavior is:

- Keep this hook focused on exposing raw reward-related state.
- Expose the relevant bit (`onlyMembers`) and let the caller (or `usePoolOpenStatus`) decide how to interpret “open”.

For example:

```ts
const ubiOnlyMembers = ubiSettings ? ubiSettings[6] : undefined; // onlyMembers at index 6

// If you must keep isPoolOpen for now, make it a thin derivation from onlyMembers:
const isPoolOpen = useMemo(
  () => (poolType === 'UBI' ? (ubiOnlyMembers === undefined ? undefined : !ubiOnlyMembers) : undefined),
  [poolType, ubiOnlyMembers],
);

return {
  isPoolOpen,
  eligibleAmount: eligibleAmount ? BigInt(eligibleAmount.toString()) : 0n,
  hasClaimed: hasClaimed ?? false,
  nextClaimTime: nextClaimTime ? Number(BigInt(nextClaimTime.toString())) : undefined,
  claimPeriodDays: ubiSettings ? Number(ubiSettings[1]) : undefined,
  onlyMembers: ubiOnlyMembers,
};
```

Then, in places where you already use `usePoolOpenStatus`, you can prefer that as the single source of truth and gradually stop depending on `isPoolOpen` from this hook. That separation (one hook for rewards, one for open status) will make future changes around pool gating much easier to reason about.
</issue_to_address>

### Comment 4
<location> `packages/app/src/components/ClaimRewardButton.tsx:26` </location>
<code_context>
+  const { claimReward, isConfirming, isSuccess, isError, error } = useClaimReward(poolAddress, poolType);
+  const { eligibleAmount, hasClaimed, nextClaimTime, claimPeriodDays } = usePoolRewards(poolAddress, poolType);
+  const { price: tokenPrice } = useGetTokenPrice('G$');
+  const [showClaimModal, setShowClaimModal] = useState(false);
+  const [showProcessingModal, setShowProcessingModal] = useState(false);
+  const [showSuccessModal, setShowSuccessModal] = useState(false);
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring the claim flow to use a single status-driven modal state and an optional container component so the button logic is simpler and more declarative.

You can simplify this flow quite a bit by:

1) Replacing the four modal booleans + error string with a single status state
2) Centralizing error handling to use a single source of truth
3) (Optional) Extracting the reward-fetching/timer logic into a tiny wrapper so this component only deals with the claim transaction and modals

Here’s how you could do (1) and (2) in a focused way.

1) Use a single status instead of multiple booleans

Right now you’re juggling:

- showClaimModal
- showProcessingModal
- showSuccessModal
- errorMessage (plus isError / isSuccess / isConfirming)

You can replace these with a single status state that drives which modal is shown:

```ts
type ClaimStatus = 'idle' | 'confirm' | 'processing' | 'success' | 'error';

const [status, setStatus] = useState<ClaimStatus>('idle');
const [errorMessage, setErrorMessage] = useState<string | undefined>();

const handleClaimClick = () => {
  setStatus('confirm');
};

const handleConfirmClaim = async () => {
  setStatus('processing');
  try {
    await claimReward();
    // success is handled by useEffect listening to isSuccess
  } catch (err) {
    const message = err instanceof Error ? err.message : 'Failed to claim reward';
    setErrorMessage(message);
    setStatus('error');
  }
};
```

Rendering becomes simpler and declarative:

```tsx
<BaseModal
  openModal={status === 'confirm'}
  onClose={() => setStatus('idle')}
  onConfirm={handleConfirmClaim}
  // ...
/>

<BaseModal
  openModal={status === 'processing'}
  onClose={() => {}}
  // ...
/>

<BaseModal
  openModal={status === 'success'}
  onClose={() => setStatus('idle')}
  onConfirm={() => setStatus('idle')}
  // ...
/>

<BaseModal
  type="error"
  openModal={status === 'error'}
  onClose={() => { setErrorMessage(undefined); setStatus('idle'); }}
  onConfirm={() => { setErrorMessage(undefined); setStatus('idle'); }}
  errorMessage={errorMessage ?? ''}
/>
```

2) Centralize success/error handling (remove duplication)

Currently:

- handleConfirmClaim catch block sets processing false and errorMessage
- useEffect on isError also sets processing false and errorMessage
- useEffect on isSuccess + !isConfirming also controls state

Pick one path for error handling to avoid duplication. For example, rely only on the hook state (no try/catch state changes) and keep try/catch just for unexpected synchronous errors:

```ts
const handleConfirmClaim = async () => {
  setStatus('processing');
  try {
    await claimReward();
  } catch (err) {
    // Only handle truly unexpected errors here
    const message = err instanceof Error ? err.message : 'Failed to claim reward';
    setErrorMessage(message);
    setStatus('error');
  }
};

useEffect(() => {
  if (isSuccess && !isConfirming) {
    setStatus('success');
    onSuccess?.();
  }
}, [isSuccess, isConfirming, onSuccess]);

useEffect(() => {
  if (isError && error) {
    const message = error.message || 'Failed to claim reward';
    setErrorMessage(message);
    setStatus('error');
  }
}, [isError, error]);
```

This removes:

- Manual toggling of showProcessingModal/showSuccessModal in multiple places
- Duplication of “processing → error” transitions

All modal visibility is now derived from `status`, and success/error transitions are handled in one place each.

3) Optional: Extract reward-fetching/timer logic

If you want to further reduce density, you can wrap this component with a container that handles the `usePoolRewards` logic and only renders `ClaimRewardButton` when it should be clickable:

```tsx
// ClaimRewardButtonContainer.tsx
export const ClaimRewardButtonContainer: React.FC<ClaimRewardButtonProps> = (props) => {
  const { eligibleAmount, hasClaimed, nextClaimTime, claimPeriodDays } = usePoolRewards(props.poolAddress, props.poolType);

  if (hasClaimed && nextClaimTime && claimPeriodDays) {
    return <ClaimTimer nextClaimTime={nextClaimTime} claimPeriodDays={claimPeriodDays} poolName={props.poolName} />;
  }

  return (
    <ClaimRewardButton
      {...props}
      eligibleAmount={eligibleAmount}
      // pass what ClaimRewardButton actually needs
    />
  );
};
```

Then `ClaimRewardButton` only needs to deal with:

- The claim transaction (`useClaimReward`)
- The modal flow driven by `status`

This keeps all existing behavior but makes each piece easier to follow and maintain.
</issue_to_address>

### Comment 5
<location> `packages/app/src/components/JoinPoolButton.tsx:17` </location>
<code_context>
+  onSuccess?: () => void;
+}
+
+export const JoinPoolButton: React.FC<JoinPoolButtonProps> = ({ poolAddress, poolType, poolName, onSuccess }) => {
+  const { address } = useAccount();
+  const { joinPool, isConfirming, isSuccess, isError, error, hash } = useJoinPool(poolAddress, poolType);
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the shared transaction + modal lifecycle into a reusable hook and wrapper so JoinPoolButton (and similar buttons) only handle wiring and copy text.

You can reduce complexity and future duplication by extracting the repeated “transaction + modal flow” pattern into a small reusable hook + lightweight wrapper component, and then using that inside `JoinPoolButton`. This will also make it easier to keep `JoinPoolButton` and `ClaimRewardButton` behavior in sync.

For example, you can encapsulate the modal state and transaction lifecycle into a generic hook:

```ts
// useTransactionWithModals.ts
import { useEffect, useState } from 'react';

interface UseTransactionWithModalsParams {
  runTransaction: () => Promise<void>;
  isSuccess: boolean;
  isConfirming: boolean;
  isError: boolean;
  error?: Error | null;
  hash?: `0x${string}` | null;
  onSuccess?: () => void;
  successDelayMs?: number;
  defaultErrorMessage?: string;
}

export const useTransactionWithModals = ({
  runTransaction,
  isSuccess,
  isConfirming,
  isError,
  error,
  hash,
  onSuccess,
  successDelayMs = 1000,
  defaultErrorMessage = 'Transaction failed',
}: UseTransactionWithModalsParams) => {
  const [showConfirmModal, setShowConfirmModal] = useState(false);
  const [showProcessingModal, setShowProcessingModal] = useState(false);
  const [showSuccessModal, setShowSuccessModal] = useState(false);
  const [errorMessage, setErrorMessage] = useState<string | undefined>();

  const openConfirm = () => setShowConfirmModal(true);

  const confirmAndRun = async () => {
    setShowConfirmModal(false);
    setShowProcessingModal(true);
    try {
      await runTransaction();
    } catch (err) {
      setShowProcessingModal(false);
      const message =
        err instanceof Error ? err.message : defaultErrorMessage;
      setErrorMessage(message);
    }
  };

  useEffect(() => {
    if (isSuccess && !isConfirming && hash) {
      setShowProcessingModal(false);
      setShowSuccessModal(true);
      setTimeout(() => {
        onSuccess?.();
      }, successDelayMs);
    }
  }, [isSuccess, isConfirming, hash, onSuccess, successDelayMs]);

  useEffect(() => {
    if (isError && error) {
      setShowProcessingModal(false);
      const message = error.message || defaultErrorMessage;
      setErrorMessage(message);
    }
  }, [isError, error, defaultErrorMessage]);

  return {
    // modal state
    showConfirmModal,
    showProcessingModal,
    showSuccessModal,
    errorMessage,

    // modal controls
    openConfirm,
    confirmAndRun,
    closeSuccess: () => setShowSuccessModal(false),
    clearError: () => setErrorMessage(undefined),
  };
};
```

Then `JoinPoolButton` becomes mostly wiring + copy for the modals, with the transaction flow centralized:

```tsx
// JoinPoolButton.tsx
export const JoinPoolButton: React.FC<JoinPoolButtonProps> = ({
  poolAddress,
  poolType,
  poolName,
  onSuccess,
}) => {
  const { address } = useAccount();
  const {
    joinPool,
    isConfirming,
    isSuccess,
    isError,
    error,
    hash,
  } = useJoinPool(poolAddress, poolType);

  const {
    showConfirmModal,
    showProcessingModal,
    showSuccessModal,
    errorMessage,
    openConfirm,
    confirmAndRun,
    closeSuccess,
    clearError,
  } = useTransactionWithModals({
    runTransaction: joinPool,
    isSuccess,
    isConfirming,
    isError,
    error,
    hash,
    onSuccess,
    defaultErrorMessage: 'Failed to join pool',
  });

  if (!address) return null;

  return (
    <View>
      <RoundedButton
        title="Join Pool"
        backgroundColor={Colors.green[100]}
        color={Colors.green[200]}
        onPress={openConfirm}
      />

      <BaseModal
        openModal={showConfirmModal}
        onClose={() => {}}
        onConfirm={confirmAndRun}
        title="JOIN POOL"
        paragraphs={[`To join ${poolName || 'this pool'}, please sign with your wallet.`]}
        image={PhoneImg}
        confirmButtonText="JOIN"
      />

      <BaseModal
        openModal={showProcessingModal}
        onClose={() => {}}
        title="PROCESSING"
        paragraphs={['Please wait while we process your request...']}
        image={ApproveTokenImg}
        withClose={false}
      />

      <BaseModal
        openModal={showSuccessModal}
        onClose={closeSuccess}
        onConfirm={closeSuccess}
        title="SUCCESS!"
        paragraphs={[`You have successfully joined ${poolName || 'the pool'}!`]}
        image={ThankYouImg}
        confirmButtonText="OK"
      />

      <BaseModal
        type="error"
        openModal={!!errorMessage}
        onClose={clearError}
        onConfirm={clearError}
        errorMessage={errorMessage ?? ''}
      />
    </View>
  );
};
```

Once this hook/shape is in place, `ClaimRewardButton` (and any future transactional buttons) can reuse `useTransactionWithModals` with different:

- `runTransaction` function (e.g., `claimReward`)
- Copy (titles, paragraphs, success text)
- Default error message

This keeps all transaction lifecycle logic and modal state transitions in one place, reducing complexity and maintenance overhead while preserving current behavior.
</issue_to_address>

### Comment 6
<location> `packages/app/src/hooks/useJoinPool.ts:18` </location>
<code_context>
+  },
+] as const;
+
+export function useJoinPool(poolAddress: `0x${string}` | undefined, poolType?: string) {
+  const { address, chain } = useAccount();
+
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the shared simulate→write→wait logic into a reusable hook so useJoinPool (and similar hooks) become thin, consistent wrappers with less duplicated code.

You can reduce complexity and duplication here by extracting the shared “simulate → write → wait → merged error” pattern into a small reusable hook, then make `useJoinPool` a thin wrapper around it.

For example, introduce a generic helper:

```ts
// useSimulatedWrite.ts
import {
  useSimulateContract,
  useWriteContract,
  useWaitForTransactionReceipt,
} from 'wagmi';

type UseSimulatedWriteParams<TAbi, TFunctionName extends string> = {
  chainId?: number;
  address?: `0x${string}`;
  abi: TAbi;
  functionName: TFunctionName;
  args: unknown[];
  enabled: boolean;
};

export function useSimulatedWrite<TAbi, TFunctionName extends string>({
  chainId,
  address,
  abi,
  functionName,
  args,
  enabled,
}: UseSimulatedWriteParams<TAbi, TFunctionName>) {
  const { data: simulateData, error: simulateError } = useSimulateContract({
    chainId,
    address,
    abi,
    functionName,
    args,
    query: { enabled },
  });

  const {
    writeContractAsync,
    isPending,
    isError,
    error,
    data: hash,
  } = useWriteContract();

  const { isLoading: isConfirming, isSuccess } = useWaitForTransactionReceipt({
    hash,
    chainId,
  });

  async function execute() {
    if (!simulateData) {
      throw new Error('Transaction simulation failed');
    }
    return writeContractAsync(simulateData.request);
  }

  return {
    execute,
    isPending,
    isConfirming,
    isSuccess,
    isError,
    error: error || simulateError,
    hash,
  };
}
```

Then `useJoinPool` becomes mostly configuration, which also makes it easier to keep behavior consistent with `useClaimReward`:

```ts
import { useAccount } from 'wagmi';
import { useSimulatedWrite } from './useSimulatedWrite';

export function useJoinPool(
  poolAddress: `0x${string}` | undefined,
  poolType?: string,
) {
  const { address, chain } = useAccount();
  const isUBIPool = poolType === 'UBI';

  const {
    execute,
    isPending,
    isConfirming,
    isSuccess,
    isError,
    error,
    hash,
  } = useSimulatedWrite({
    chainId: chain?.id,
    address: poolAddress,
    abi: UBI_POOL_ABI,
    functionName: 'addMember',
    args: [address as `0x${string}`, '0x' as `0x${string}`],
    enabled: !!poolAddress && !!address && isUBIPool,
  });

  return {
    joinPool: execute,
    isPending,
    isConfirming,
    isSuccess,
    isError,
    error,
    hash,
  };
}
```

You can apply the same pattern to `useClaimReward` (and any future transaction hooks) so that changes to error handling, logging, gas tracking, etc. only need to be made in `useSimulatedWrite`.
</issue_to_address>

### Comment 7
<location> `packages/app/src/hooks/useClaimReward.ts:15` </location>
<code_context>
+  },
+] as const;
+
+export function useClaimReward(poolAddress: `0x${string}` | undefined, poolType: string | undefined) {
+  const { address, chain } = useAccount();
+
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the shared simulate→write→wait transaction flow into a reusable hook and making useClaimReward a thin wrapper over it.

You now have (at least) two hooks (this one and `useJoinPool`) that locally reimplement the same “simulate → write → wait” pattern. That duplication will make future changes to transaction behavior harder to reason about (e.g., error handling, pending/confirming states, enabling logic).

You can reduce complexity by extracting a shared hook for “simulated writes” and then make `useClaimReward` a thin wrapper around it.

For example, introduce a generic hook:

```ts
// useSimulatedWrite.ts
import {
  useAccount,
  useSimulateContract,
  useWriteContract,
  useWaitForTransactionReceipt,
} from 'wagmi';

export function useSimulatedWrite<
  TAbi extends readonly unknown[],
  TFunctionName extends string,
>({
  chainId,
  address,
  abi,
  functionName,
  args,
  enabled,
}: {
  chainId?: number;
  address?: `0x${string}`;
  abi: TAbi;
  functionName: TFunctionName;
  args?: unknown[];
  enabled: boolean;
}) {
  const { data: simulateData, error: simulateError } = useSimulateContract({
    chainId,
    address,
    abi,
    functionName,
    args,
    query: { enabled },
  });

  const { writeContractAsync, isPending, isError, error, data: hash } =
    useWriteContract();

  const { isLoading: isConfirming, isSuccess } = useWaitForTransactionReceipt({
    hash,
    chainId,
  });

  const write = async () => {
    if (!simulateData) {
      throw new Error('Transaction simulation failed');
    }
    return writeContractAsync(simulateData.request);
  };

  return {
    write,
    isPending,
    isConfirming,
    isSuccess,
    isError,
    error: error || simulateError,
    hash,
  };
}
```

Then `useClaimReward` becomes very small and descriptive:

```ts
const UBI_POOL_CLAIM_ABI = [
  {
    inputs: [],
    name: 'claim',
    outputs: [],
    stateMutability: 'nonpayable',
    type: 'function',
  },
] as const;

export function useClaimReward(
  poolAddress: `0x${string}` | undefined,
  poolType: string | undefined,
) {
  const { address, chain } = useAccount();
  const enabled = !!poolAddress && !!address && poolType === 'UBI';

  const {
    write: claimReward,
    isPending,
    isConfirming,
    isSuccess,
    isError,
    error,
    hash,
  } = useSimulatedWrite({
    chainId: chain?.id,
    address: poolAddress,
    abi: UBI_POOL_CLAIM_ABI,
    functionName: 'claim',
    enabled,
  });

  return {
    claimReward,
    isPending,
    isConfirming,
    isSuccess,
    isError,
    error,
    hash,
  };
}
```

You can then migrate `useJoinPool` (and any other similar hooks) to use `useSimulatedWrite` as well, so that:

- Transaction state handling (pending/confirming/success/error) is defined once.
- Simulation failure handling is consistent.
- Per-action hooks stay small and focused on domain-specific details (ABI, function name, enabled conditions).
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[L03TJ3]

Unncessary hook.
Look at the goodcollective-sdk
it already provides helpers to obtain all of this data.

look at getMemberUBIPools
it returns a list of all pools someone is member off.
so:

1. needs to be filtered to the current collective/pool someone is on the page for
2. hasClaimed is not returned through the sdk, but you can just verify claimAmount === 0 (0 means claimed)

should replace usage of this hook in ClaimRewardButton with using the sdk helper.
I think to even see that sdk.getMemberUBIPools should be run from 'ViewCollective.tsx'
and the relevant data passed down to hooks or components from there.

[L03TJ3]

See other comment, replace these two hooks with usage of sdk.getMemberUbiPools

[L03TJ3]

1. this comment is not followed up on
2. isPoolOpen, no need. just use memberPoolData?.onlyMembers
3. isMember, redundant. getMemberUBIPools returns a list of all pools a connected wallet is member of. so using your new useEffect this can be removed and replace isMember with !memberPoolData (since its null when no pools are found for the connected wallet)

[L03TJ3]

The ABI as is in deployment.json should always be used and leading (might change in the future)
so:

import GoodCollectiveContracts from '@gooddollar/goodcollective-contracts'

And

const networkName = env.REACT_APP_NETWORK || 'development-celo';
const abi = GoodCollectiveContracts["42220"]?.find(envs => envs.name === networkName)?.contracts.UBIPool?.abi;

[L03TJ3]

See other comment about ABI. import from goodcollective-contracts package

[L03TJ3]

1. this comment is not followed up on
2. isPoolOpen, no need. just use memberPoolData?.onlyMembers
3. isMember, redundant. getMemberUBIPools returns a list of all pools a connected wallet is member of. so using your new useEffect this can be removed and replace isMember with !memberPoolData (since its null when no pools are found for the connected wallet)

[L03TJ3]

Comments should be placed to explain something complex, solving an edge case, instruction to another developer (or for future reference for yourself) why something has been done a certain way (solving a specific issue, bug, that required unusual handling)

for the rest, if code is self-explanatory, over commenting just obfuscates the code

[L03TJ3]

Besides the reported bug in error handling (trying to join a pool when someone is not whitelisted).

Also verify if you check maxMembers, and if this is reach (another reason a 'join-pool' transactions gets reverted)

See addMember and grantRole in the UBI.sol

1. create an error mapping for NOT_WHITELISTED and MAX_MEMBERS_REACHED

• Error mapping with a copy/text that can be displayed to the user.

const joinPoolErrors = {
NOT_WHITELISTED: 'You need to be a whitelisted G$ user to join this pool. Visit <link to https://gooddapp.org> and verify yourself by claiming your first G$'s.',
MAX_MEMBERS_REACHED: 'This pool reached the maximum amount of members.'
}

2. Even though max-members-reached should be verified before the join button is shown. perhaps multiple people join at the same time so it could still occur, so we still want to handle the revert error.

[L03TJ3]

If the claim amount is 0 (can happen if the pool runs out of funds)
we should not show the Claim G$ button.

• if claim amount is 0, hide everything. no join button, no timer.