Skip to content
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

feat: re-type walletCapabilities object #1238

Merged
merged 13 commits into from
Sep 16, 2024
Merged
5 changes: 5 additions & 0 deletions .changeset/tough-books-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@coinbase/onchainkit': patch
---

**feat**: re-typed walletCapabilities object in `OnchainKitConfig`. by @0xAlec #1238
5 changes: 0 additions & 5 deletions src/OnchainKitConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ export const ONCHAIN_KIT_CONFIG: OnchainKitConfig = {
chain: baseSepolia,
rpcUrl: null,
schemaId: null,
walletCapabilities: {
hasAtomicBatch: false,
hasAuxiliaryFunds: false,
hasPaymasterService: false,
},
};

/**
Expand Down
81 changes: 3 additions & 78 deletions src/OnchainKitProvider.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { act, render, screen, waitFor } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import { base } from 'viem/chains';
import '@testing-library/jest-dom';

Expand All @@ -11,7 +11,6 @@ import { mock } from 'wagmi/connectors';
import { setOnchainKitConfig } from './OnchainKitConfig';
import { OnchainKitProvider } from './OnchainKitProvider';
import type { EASSchemaUid } from './identity/types';
import { useCapabilitiesSafe } from './useCapabilitiesSafe';
import { useOnchainKit } from './useOnchainKit';

const queryClient = new QueryClient();
Expand All @@ -26,7 +25,6 @@ const mockConfig = createConfig({
[base.id]: http(),
},
});
vi.mock('./useCapabilitiesSafe');

const TestComponent = () => {
const { schemaId, apiKey } = useOnchainKit();
Expand All @@ -46,13 +44,9 @@ vi.mock('./OnchainKitConfig', () => ({
chain: base,
rpcUrl: null,
schemaId: null,
walletCapabilities: {
hasPaymasterService: false,
hasAtomicBatch: false,
hasAuxiliaryFunds: false,
},
},
}));

describe('OnchainKitProvider', () => {
const schemaId: EASSchemaUid = `0x${'1'.repeat(64)}`;
const apiKey = 'test-api-key';
Expand Down Expand Up @@ -116,7 +110,7 @@ describe('OnchainKitProvider', () => {
}).not.toThrow();
});

it('should call setOnchainKitConfig with the correct default values', async () => {
it('should call setOnchainKitConfig with the correct values', async () => {
render(
<WagmiProvider config={mockConfig}>
<QueryClientProvider client={queryClient}>
Expand All @@ -133,75 +127,6 @@ describe('OnchainKitProvider', () => {
chain: base,
rpcUrl: null,
schemaId,
walletCapabilities: {
hasPaymasterService: false,
hasAtomicBatch: false,
hasAuxiliaryFunds: false,
},
});
});

it('should call setOnchainKitConfig when capabilities are found', async () => {
const walletCapabilities = {
hasPaymasterService: true,
hasAtomicBatch: true,
hasAuxiliaryFunds: true,
};
vi.mocked(useCapabilitiesSafe).mockReturnValue(walletCapabilities);
await act(async () => {
render(
<WagmiProvider config={mockConfig}>
<QueryClientProvider client={queryClient}>
<OnchainKitProvider
chain={base}
schemaId={schemaId}
apiKey={apiKey}
>
<TestComponent />
</OnchainKitProvider>
</QueryClientProvider>
</WagmiProvider>,
);
});
expect(setOnchainKitConfig).toHaveBeenCalledWith({
address: null,
apiKey,
chain: base,
rpcUrl: null,
schemaId,
walletCapabilities,
});
});

it('should call setOnchainKitConfig when capabilities are not found', async () => {
const walletCapabilities = {
hasPaymasterService: false,
hasAtomicBatch: false,
hasAuxiliaryFunds: false,
};
vi.mocked(useCapabilitiesSafe).mockReturnValue(walletCapabilities);
await act(async () => {
render(
<WagmiProvider config={mockConfig}>
<QueryClientProvider client={queryClient}>
<OnchainKitProvider
chain={base}
schemaId={schemaId}
apiKey={apiKey}
>
<TestComponent />
</OnchainKitProvider>
</QueryClientProvider>
</WagmiProvider>,
);
});
expect(setOnchainKitConfig).toHaveBeenCalledWith({
address: null,
apiKey,
chain: base,
rpcUrl: null,
schemaId,
walletCapabilities,
});
});
});
9 changes: 1 addition & 8 deletions src/OnchainKitProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createContext, useMemo } from 'react';
import { ONCHAIN_KIT_CONFIG, setOnchainKitConfig } from './OnchainKitConfig';
import { checkHashLength } from './internal/utils/checkHashLength';
import type { OnchainKitContextType, OnchainKitProviderReact } from './types';
import { useCapabilitiesSafe } from './useCapabilitiesSafe';

export const OnchainKitContext =
createContext<OnchainKitContextType>(ONCHAIN_KIT_CONFIG);
Expand All @@ -21,7 +20,6 @@ export function OnchainKitProvider({
if (schemaId && !checkHashLength(schemaId, 64)) {
throw Error('EAS schemaId must be 64 characters prefixed with "0x"');
}
const walletCapabilities = useCapabilitiesSafe({ chainId: chain.id });

const value = useMemo(() => {
const onchainKitConfig = {
Expand All @@ -30,15 +28,10 @@ export function OnchainKitProvider({
chain: chain,
rpcUrl: rpcUrl ?? null,
schemaId: schemaId ?? null,
walletCapabilities: walletCapabilities ?? {
hasAtomicBatch: false,
hasAuxiliaryFunds: false,
hasPaymasterService: false,
},
};
setOnchainKitConfig(onchainKitConfig);
return onchainKitConfig;
}, [address, apiKey, chain, rpcUrl, schemaId, walletCapabilities]);
}, [address, apiKey, chain, rpcUrl, schemaId]);
return (
<OnchainKitContext.Provider value={value}>
{children}
Expand Down
6 changes: 6 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Capabilities
export enum Capabilities {
AtomicBatch = 'atomicBatch',
AuxiliaryFunds = 'auxiliaryFunds',
PaymasterService = 'paymasterService',
}
Copy link
Contributor Author

@0xAlec 0xAlec Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll still need to use this enum for any capabilities we support in OnchainKit but now developers can use non-OCK supported capabilities since it isn't custom typed anymore

const { walletCapabilities } = useOnchainKit()

if (walletCapabilities[someOtherCapability]){
  ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each of them, we should probably add a comment that links on where the EIP is.

Just because I can imagine a convo in 2 months where someone else tries to figure out what those are and they take a few to find the correct docs.

1 change: 0 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ export type {
OnchainKitConfig,
OnchainKitContextType,
OnchainKitProviderReact,
WalletCapabilities,
} from './types';
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@ vi.mock('wagmi/experimental', () => ({
describe('useCapabilitiesSafe', () => {
const mockChainId = 1;
const walletCapabilitiesTrue = {
hasAtomicBatch: true,
hasAuxiliaryFunds: true,
hasPaymasterService: true,
};
const walletCapabilitiesFalse = {
hasAtomicBatch: false,
hasAuxiliaryFunds: false,
hasPaymasterService: false,
atomicBatch: { supported: true },
paymasterService: { supported: true },
auxiliaryFunds: { supported: true },
};
const walletCapabilitiesFalse = {};

beforeEach(() => {
vi.resetAllMocks();
Expand Down
23 changes: 23 additions & 0 deletions src/internal/hooks/useCapabilitiesSafe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { useMemo } from 'react';
import type { WalletCapabilities } from 'viem';
import { useAccount } from 'wagmi';
import { useCapabilities } from 'wagmi/experimental';
import type { UseCapabilitiesSafeParams } from '../../types';

export function useCapabilitiesSafe({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still feel like we should rename this useCapabilities, as the fact that we are not exporting this externally adding the word Safe just feels odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will make more sense once we add the functionality to debounce or stop subsequent calls to non-EIP5792 wallets on this hook

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(which isn't supported in the vanilla Wagmi hook - hence unsafe)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also say the vanilla Wagmi hook is unsafe because it does not return an empty object for non-EIP5792 wallets

chainId,
}: UseCapabilitiesSafeParams): WalletCapabilities {
const { isConnected } = useAccount();

const { data: capabilities, error } = useCapabilities({
query: { enabled: isConnected },
});

return useMemo(() => {
if (error || !capabilities || !capabilities[chainId]) {
return {};
}

return capabilities[chainId];
}, [capabilities, chainId, error]);
}
46 changes: 20 additions & 26 deletions src/transaction/components/TransactionProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
useWaitForTransactionReceipt,
} from 'wagmi';
import { waitForTransactionReceipt } from 'wagmi/actions';
import { useOnchainKit } from '../../useOnchainKit';
import { useCapabilitiesSafe } from '../../internal/hooks/useCapabilitiesSafe';
import { useCallsStatus } from '../hooks/useCallsStatus';
import { useSendCall } from '../hooks/useSendCall';
import { useSendCalls } from '../hooks/useSendCalls';
Expand Down Expand Up @@ -55,8 +55,8 @@ vi.mock('../hooks/useSendWalletTransactions', () => ({
useSendWalletTransactions: vi.fn(),
}));

vi.mock('../../useOnchainKit', () => ({
useOnchainKit: vi.fn(),
vi.mock('../../internal/hooks/useCapabilitiesSafe', () => ({
useCapabilitiesSafe: vi.fn(),
}));

const silenceError = () => {
Expand Down Expand Up @@ -156,13 +156,7 @@ describe('TransactionProvider', () => {
(useWaitForTransactionReceipt as ReturnType<typeof vi.fn>).mockReturnValue({
receipt: undefined,
});
(useOnchainKit as ReturnType<typeof vi.fn>).mockReturnValue({
walletCapabilities: {
hasAtomicBatch: false,
hasPaymasterService: false,
hasAuxiliaryFunds: false,
},
});
(useCapabilitiesSafe as ReturnType<typeof vi.fn>).mockReturnValue({});
});

it('should emit onError when setLifeCycleStatus is called with error', async () => {
Expand Down Expand Up @@ -293,10 +287,10 @@ describe('TransactionProvider', () => {
status: 'pending',
writeContractsAsync: writeContractsAsyncMock,
});
(useOnchainKit as ReturnType<typeof vi.fn>).mockReturnValue({
walletCapabilities: {
hasAtomicBatch: true,
},
(useCapabilitiesSafe as ReturnType<typeof vi.fn>).mockReturnValue({
atomicBatch: { supported: true },
paymasterService: { supported: true },
auxiliaryFunds: { supported: true },
});
render(
<TransactionProvider contracts={[]}>
Expand Down Expand Up @@ -336,10 +330,10 @@ describe('TransactionProvider', () => {

it('should update context on handleSubmit', async () => {
const sendWalletTransactionsMock = vi.fn();
(useOnchainKit as ReturnType<typeof vi.fn>).mockReturnValue({
walletCapabilities: {
hasAtomicBatch: true,
},
(useCapabilitiesSafe as ReturnType<typeof vi.fn>).mockReturnValue({
atomicBatch: { supported: true },
paymasterService: { supported: true },
auxiliaryFunds: { supported: true },
});
(useSendWalletTransactions as ReturnType<typeof vi.fn>).mockReturnValue(
sendWalletTransactionsMock,
Expand All @@ -364,10 +358,10 @@ describe('TransactionProvider', () => {
status: 'idle',
writeContractsAsync: writeContractsAsyncMock,
});
(useOnchainKit as ReturnType<typeof vi.fn>).mockReturnValue({
walletCapabilities: {
hasAtomicBatch: true,
},
(useCapabilitiesSafe as ReturnType<typeof vi.fn>).mockReturnValue({
atomicBatch: { supported: true },
paymasterService: { supported: true },
auxiliaryFunds: { supported: true },
});
render(
<TransactionProvider contracts={[]}>
Expand Down Expand Up @@ -435,10 +429,10 @@ describe('TransactionProvider', () => {
(useSendWalletTransactions as ReturnType<typeof vi.fn>).mockReturnValue(
sendWalletTransactionsMock,
);
(useOnchainKit as ReturnType<typeof vi.fn>).mockReturnValue({
walletCapabilities: {
hasAtomicBatch: true,
},
(useCapabilitiesSafe as ReturnType<typeof vi.fn>).mockReturnValue({
atomicBatch: { supported: true },
paymasterService: { supported: true },
auxiliaryFunds: { supported: true },
});
render(
<TransactionProvider contracts={[]}>
Expand Down
13 changes: 9 additions & 4 deletions src/transaction/components/TransactionProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ import {
useState,
} from 'react';
import type { Address } from 'viem';
import { baseSepolia } from 'viem/chains';
import {
useAccount,
useConfig,
useSwitchChain,
useWaitForTransactionReceipt,
} from 'wagmi';
import { waitForTransactionReceipt } from 'wagmi/actions';
import { Capabilities } from '../../constants';
import { useCapabilitiesSafe } from '../../internal/hooks/useCapabilitiesSafe';
import { useValue } from '../../internal/hooks/useValue';
import { useOnchainKit } from '../../useOnchainKit';
import {
GENERIC_ERROR_MESSAGE,
TRANSACTION_TYPE_CALLS,
Expand Down Expand Up @@ -77,7 +79,9 @@ export function TransactionProvider({
: TRANSACTION_TYPE_CONTRACTS;

// Retrieve wallet capabilities
const { walletCapabilities } = useOnchainKit();
const walletCapabilities = useCapabilitiesSafe({
chainId: chainId || baseSepolia.id,
Copy link
Contributor

@Zizzamia Zizzamia Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make sure we handle this correctly where:

  • we prioritize the chainId from the Transaction component
  • if that not present we use the chainId from useOnchainKit()
  • which if not present at OnchainKit config defaults to BaseSepolia.

We can not merge this PR, if this is not present! Just FYI
cc @0xAlec @alessey @cpcramer @abcrane123

}); // defaults to Base Sepolia if not provided

const { switchChainAsync } = useSwitchChain();

Expand Down Expand Up @@ -127,7 +131,8 @@ export function TransactionProvider({
// For batched, use statusSendCalls or statusWriteContracts
// For single, use statusSendCall or statusWriteContract
const transactionStatus = useMemo(() => {
const transactionStatuses = walletCapabilities.hasAtomicBatch
const transactionStatuses = walletCapabilities[Capabilities.AtomicBatch]
?.supported
? {
[TRANSACTION_TYPE_CALLS]: statusSendCalls,
[TRANSACTION_TYPE_CONTRACTS]: statusWriteContracts,
Expand All @@ -143,7 +148,7 @@ export function TransactionProvider({
statusSendCall,
statusWriteContract,
transactionType,
walletCapabilities.hasAtomicBatch,
walletCapabilities[Capabilities.AtomicBatch],
]);

// Transaction hash for single transaction (non-batched)
Expand Down
Loading