Conversation
- Updated `package.json` to use `tsup` for building and added module exports. - Changed TypeScript module resolution to `bundler` and updated module target to `ES2022`. - Introduced `tsup.config.ts` for build configuration.
- Introduced a new constant `CHAIN_IDS` to define common chain IDs for various Celo networks, including CELO_SEPOLIA and ALFAJORES, to facilitate contract client creation.
- Added a new `index.ts` file to define the `createOakContractsClient` function, which supports both simple and full configuration for creating a client instance. - Included utility functions for client configuration and chain resolution, along with type guards for better type safety. - The client allows interaction with various contract entities such as global parameters, campaign info, and treasury factories.
- Expanded `client.ts` with new interfaces for campaign data, payment structures, and treasury configurations to improve type safety and clarity. - Consolidated protocol types in `index.ts` to streamline exports and reduce circular dependencies. - Introduced `SimpleOakContractsClientConfig` and `FullOakContractsClientConfig` for flexible client configuration options.
- Introduced Celo Sepolia testnet to the chain registry, enhancing support for Celo networks. - Updated import paths for `defineChain`, `mainnet`, `sepolia`, and `goerli` to use the new structure from the `viem` package. - Re-exported `CHAIN_IDS` from constants for simplified client configuration.
- Removed the `client.ts` file as part of the restructuring. - Added multiple new contract entities including `AllOrNothing`, `CampaignInfoFactory`, `CampaignInfo`, `GlobalParams`, `ItemRegistry`, `KeepWhatsRaised`, `PaymentTreasury`, and `TreasuryFactory` to enhance functionality and modularity. - Updated `index.ts` to export the new contract entities for easier access. - Improved type safety and clarity in contract interactions by defining specific entity functions for read/write operations.
- Introduced a comprehensive README.md file for the @oaknetwork/contracts package. - Documented installation instructions, quick start guide, client configuration options, supported chain IDs, and detailed contract functionalities. - Enhanced clarity and usability for developers interacting with Oak Network smart contracts.
- Enhanced `AllOrNothing` ABI by adding new functions `cancelTreasury` and `cancelled`, and modified event names and input types for better clarity. - Updated `CampaignInfo` ABI to include the `_cancelCampaign` function and removed outdated ownership transfer events. - Refined `KeepWhatsRaised` ABI by adding `cancelTreasury` and `cancelled` functions, and corrected event names for consistency. - Adjusted `PaymentTreasury` ABI to include an `initialize` function for contract setup. - Cleaned up `GlobalParams` ABI by removing redundant input parameters for error handling functions.
- Updated `getplatformFeePercent` to `getPlatformFeePercent` in both `AllOrNothing` and `KeepWhatsRaised` contracts for consistency and adherence to naming conventions.
- Added multiple error classes for `AllOrNothing`, `CampaignInfo`, `KeepWhatsRaised`, `ItemRegistry`, `PaymentTreasury`, `Shared`, and `TreasuryFactory` to enhance error handling and provide clearer recovery hints. - Updated `index.ts` to export the new error types for better accessibility across the contracts. - Improved type safety and clarity in error management for contract interactions.
- Updated `getplatformFeePercent` to `getPlatformFeePercent` in `AllOrNothingTreasuryEntity` and `KeepWhatsRaisedTreasuryEntity` interfaces for consistency and adherence to naming conventions.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b705617608
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
devmahmud
left a comment
There was a problem hiding this comment.
Overall: Good foundational work, clean structure, thorough JSDoc, and the entity + error patterns are solid. However, there are several issues that must be addressed before merging.
| # | Severity | Issue |
|---|---|---|
| 1 | Blocker | keccak256 export collision in src/index.ts |
| 2 | Blocker | any usage in utils/index.ts |
| 3 | Blocker | cancelTreasury not in AllOrNothingTreasuryEntity |
| 4 | Blocker | createWallet passes raw hex instead of Account object |
| 5 | Major | getplatformHash / getplatformFeePercent casing inconsistency |
| 6 | Major | timeout option declared but never applied |
| 7 | Major | Zero test coverage |
| 8 | Minor | Dead ResolvedOakContractsClientConfig type |
| 9 | Minor | Deprecated GOERLI chain exported |
| 10 | Minor | Confusing entities/ vs contracts/ folder naming |
| 11 | Minor | Fragile isSimpleConfig type guard |
| 12 | Minor | No ./errors sub-path export |
| const CHAIN_REGISTRY: Record<number, Chain> = { | ||
| 1: mainnet, | ||
| 11155111: sepolia, | ||
| 5: goerli, |
…74) * fix: add missing function implementations to contract entities - Introduced `cancelled` and `cancelTreasury` functions in `AllOrNothing` and `KeepWhatsRaised` contracts. - Added `cancelCampaign` function in `CampaignInfo` contract. - Updated corresponding types in `client.ts` to reflect the new functionalities. * fix: update account handling in wallet client creation - Refactored the account assignment in the `createWallet` function to utilize `privateKeyToAccount` for improved clarity and correctness. - Removed redundant account extraction from the wallet client, streamlining the wallet creation process. * fix: standardize function name casing in PaymentTreasuryEntity - Updated function names from `getplatformHash` and `getplatformFeePercent` to `getPlatformHash` and `getPlatformFeePercent` for consistency and adherence to naming conventions in both the contract and type definitions. * refactor: remove ResolvedOakContractsClientConfig interface - Eliminated the `ResolvedOakContractsClientConfig` interface from `client.ts` to streamline the type definitions and improve clarity in client configuration handling. * refactor: enhance client configuration with timeout options - Updated the `buildClients` function to accept a new `options` parameter, allowing for customizable request timeouts in HTTP transport calls and transaction receipt waits. - Adjusted the default timeout value in `OakContractsClientOptions` from 3000ms to 30000ms for improved reliability in client operations. * feat: add error module to contract package - Introduced a new module for error handling in the contracts package by adding `errors` to the package.json and tsup.config.ts files. - Updated the build configuration to include the new `errors/index.ts` entry point for better organization and accessibility of error-related functionalities. * refactor: remove keccak256 export from contracts index - Eliminated the `keccak256` export from the `index.ts` file in the contracts package to streamline the module exports and improve clarity. * refactor: improve type definitions in wallet client functions - Updated the `createBrowserProvider` and `getSigner` functions to use `EIP1193Provider` instead of `any` for the `ethereum` parameter, enhancing type safety and clarity in the wallet client implementation. - Adjusted the transport handling in `createWallet` to utilize the `custom` transport method for better compatibility with various providers. * feat: keep Jest config loadable after switching package to ESM * refactor: update chain ID naming and usage in client configuration - Changed the chain ID from `CHAIN_IDS.CELO_SEPOLIA` to `CHAIN_IDS.CELO_TESTNET_SEPOLIA` in the client example for clarity. - Updated the chain ID constants to follow a new naming convention, distinguishing between mainnet and testnet. - Added `Celo Mainnet` definition to the chain registry for improved clarity and organization. * refactor: update chain ID references in README for clarity - Changed all instances of `CHAIN_IDS.CELO_SEPOLIA` to `CHAIN_IDS.CELO_TESTNET_SEPOLIA` in the README to reflect the correct testnet configuration. - Updated chain ID constants to better differentiate between mainnet and testnet environments, enhancing documentation accuracy. * refactor: remove deprecated entities files - It's unreferenced dead code and its functionality is already covered by `contracts/` * refactor: simplify transport handling in createWallet function * refactor: streamline transport initialization in createWallet function * refactor: enhance configuration validation in isSimpleConfig function - Improved type checks for the `chainId`, `rpcUrl`, and `privateKey` properties in the `isSimpleConfig` function to ensure they meet specific criteria, enhancing type safety and configuration reliability. * refactor: remove legacy Celo testnet constant from chain IDs - Deleted the `CELO_TESTNET_ALFAJORES` constant from the `CHAIN_IDS` to eliminate outdated references and encourage the use of `CELO_TESTNET_SEPOLIA` for new development.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2b6abdbbc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Revised chain ID documentation in README to include new mainnet and testnet identifiers for Ethereum and Celo. - Updated contract method calls to improve clarity, including renaming parameters for consistency and adding message handling for campaign and treasury functions. - Enhanced item registry methods to accept structured item data for better usability.
| import { createKeepWhatsRaisedEntity } from "../contracts/keep-whats-raised"; | ||
| import { createItemRegistryEntity } from "../contracts/item-registry"; | ||
|
|
||
| /** |
There was a problem hiding this comment.
An SDK needs proper documentation. Follow the standard on Payment SDK. Using Tsdoc.
| } as const; | ||
|
|
||
| /** | ||
| * Generates a namespaced registry key scoped to a platform (matches DataRegistryKeys.scopedToPlatform). |
There was a problem hiding this comment.
TSDOC Proper documentation.
| /** | ||
| * Resolves a chain identifier (number or Chain object) to a Chain object. | ||
| */ | ||
| function resolveChain(chain: ChainIdentifier): Chain { |
There was a problem hiding this comment.
Does this needs to be here? Isnt this a helper method?
| /** | ||
| * Builds viem publicClient and walletClient from the given config. | ||
| */ | ||
| function buildClients(config: OakContractsClientConfig, options: OakContractsClientOptions): { |
There was a problem hiding this comment.
This for me is also a helper method.
| return { | ||
| // ── Reads ──────────────────────────────────────────────────────────────── | ||
|
|
||
| async getRaisedAmount() { |
There was a problem hiding this comment.
Where is the documentation of each item?
| // ─── Shared protocol struct types ───────────────────────────────────────────── | ||
|
|
||
| /** ICampaignData.CampaignData -- used by CampaignInfo and CampaignInfoFactory */ | ||
| export interface CampaignData { |
There was a problem hiding this comment.
consider usign a better architecture, the interfaces are scathered in the code. The Campaign Data interface is a file that doesnt make sense. Maybe separated with CampaignTypes,would help? This should be easily found.
| @@ -0,0 +1,534 @@ | |||
| import type { | |||
| // ─── Entity interfaces ───────────────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * GlobalParams entity — read and write methods for a GlobalParams contract instance. |
There was a problem hiding this comment.
Guys, you are trying to solve everything in one document. This is an SDK. use responsability split,. Dont put everything in one file.
| /** | ||
| * CampaignInfo entity — full reads and writes for a deployed CampaignInfo contract. | ||
| */ | ||
| export interface CampaignInfoEntity { |
There was a problem hiding this comment.
Example... This should be in a dedicated file with their own things
| import { mainnet, sepolia, goerli } from "viem/chains"; | ||
| import type { Chain } from "viem/chains"; | ||
|
|
||
| /** Celo Mainnet */ |
Introduces the Oak Contracts SDK (
@oaknetwork/contracts): a client for interacting with Oak Network smart contracts (treasuries, campaign info, global params, etc.) with typed APIs, chain support, and structured error handling.What's included
Package & build
tsup.config.ts.bundler, targetES2022, and proper module exports inpackage.json.Client & configuration
createOakContractsClientinindex.tswith:CHAIN_IDSfor Celo networks (e.g. CELO_SEPOLIA, ALFAJORES); Celo Sepolia added to chain registry; viem imports updated for current structure.Contract entities
initialize.Types
SimpleOakContractsClientConfig,FullOakContractsClientConfig.index.tsto avoid circular deps.ABIs
cancelTreasury,cancelled, and event name/input updates._cancelCampaign; removed obsolete ownership transfer events.initialize.Error handling
Documentation
@oaknetwork/contracts: installation, quick start, client config, supported chain IDs, and contract usage.