Skip to content

feat: Add plugin framework for community extensions#101

Open
sainath5001 wants to merge 5 commits intorsksmart:mainfrom
sainath5001:feature/plugin-framework
Open

feat: Add plugin framework for community extensions#101
sainath5001 wants to merge 5 commits intorsksmart:mainfrom
sainath5001:feature/plugin-framework

Conversation

@sainath5001
Copy link

  • Implement plugin system with registry, loader, and executor
  • Create built-in plugins (transfer, balance) as examples
  • Add plugin development documentation
  • Update API route to use plugins dynamically
  • Update frontend to execute plugins generically
  • Add contribution guidelines and examples

- Implement plugin system with registry, loader, and executor
- Create built-in plugins (transfer, balance) as examples
- Add plugin development documentation
- Update API route to use plugins dynamically
- Update frontend to execute plugins generically
- Add contribution guidelines and examples
});

balance = {
displayValue: Number(queryBalance.value) / 10e18,
Copy link
Member

Choose a reason for hiding this comment

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

The value 10e18 equals 10 × 10^18 = 10^19, not 10^18. It should be 1e18. This causes balances to display 10 times smaller than the actual value.

abi: erc20Abi,
address: tokenAddress as `0x${string}`,
functionName: "transfer",
args: [address as `0x${string}`, BigInt(Math.floor(amount))],
Copy link
Member

Choose a reason for hiding this comment

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

This truncates the amount to an integer, losing all decimals. Additionally, it doesn't consider the token's decimals (some tokens have 18, others have 6, etc.).

let transactionHash: string;

if (tokenAddress === "trbtc") {
transactionHash = await sendTransaction(context.config as any, {
Copy link
Member

Choose a reason for hiding this comment

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

Multiple instances of context.config as any remove TypeScript's type safety:

address?: string;
isConnected: boolean;
config: unknown; // Wagmi config
[key: string]: unknown; // Allow additional context
Copy link
Member

Choose a reason for hiding this comment

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

Plugin context is too permissive, this allows injecting any data without validation.

address?: string;
isConnected: boolean;
config: unknown; // Wagmi config
[key: string]: unknown; // Allow additional context
Copy link
Member

Choose a reason for hiding this comment

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

I think all the // comments are not needed , please take them out (in your entire contribution)

- Add address validation in transfer plugin
- Fix TypeScript types (use Config from wagmi)
- Unify plugin registration system
- Fix balance calculation (use formatEther)
- Fix amount handling (use parseUnits with decimals)
- Add thread-safety for serverless environments
- Fix example plugin types
- Remove all comments
- Fix constants.ts typo
- Make PluginContext strict
@sainath5001
Copy link
Author

Hi, thank you for the detailed feedback! I've addressed all the issues:

  • Address validation: Added validation in transfer.ts plugin itself (lines 64-68)
  • TypeScript type safety: Removed all as any instances, now using proper Config type from wagmi
  • Unified plugin registration: Removed duplicate clientPlugins Map, using single pluginRegistry
  • Missing documentation files: Updated verify-plugins.js to remove references
  • Constants typo: Fixed contants.tsconstants.ts
  • Thread-safety: Added initLock mechanism in route.ts
  • Example plugin types: Changed to Record<string, unknown>

All changes have been committed and pushed. Ready for re-review!

src/app/page.tsx Outdated
role: "bot",
content: (
try {
// Execute plugin function dynamically
Copy link
Member

Choose a reason for hiding this comment

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

Please take out all the comments, not needed.

- Remove duplicate plugin registration in index.ts
- Add JSON.parse error handling in route.ts
- Fetch token decimals dynamically from contract
- Add React import (ReactNode) in types.ts
- Sanitize URL parameters with encodeURIComponent
- Add address validation in balance.ts
- Verify ERC20 contract before transfer
- Replace all console statements with logger
- Remove all comments from page.tsx
Copy link
Author

@sainath5001 sainath5001 left a comment

Choose a reason for hiding this comment

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

I’ve resolved the mentioned issues and updated the PR accordingly.

- Fetch token decimals from contract (consistent with transfer.ts)
- Use formatUnits instead of formatEther for ERC20 balances
- Fixes incorrect display for tokens with non-18 decimals (e.g. USDT 6 decimals)
Copy link
Author

@sainath5001 sainath5001 left a comment

Choose a reason for hiding this comment

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

Done. balance.ts now uses dynamic decimals (formatUnits + decimals from contract), consistent with transfer.ts. Thanks!

@sainath5001
Copy link
Author

Build issues resolved.
Production build tested successfully.
Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants