Feat: Introduce viem publicClient and walletClient abstraction#87
Open
Rav1Chauhan wants to merge 5 commits intoDjedAlliance:mainfrom
Open
Feat: Introduce viem publicClient and walletClient abstraction#87Rav1Chauhan wants to merge 5 commits intoDjedAlliance:mainfrom
Rav1Chauhan wants to merge 5 commits intoDjedAlliance:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Viem client factory and updates stable coin reads to use Viem publicClient when given contract addresses and an RPC URL; changes calculateFutureScPrice signature to accept addresses and rpcUrl; adds a build script and the Changes
Sequence Diagram(s)sequenceDiagram
participant StableCoinModule as "StableCoin Module"
participant ViemFactory as "createViemClients"
participant ViemPublic as "Viem publicClient"
participant OracleContract as "Oracle (contract)"
participant StableCoinContract as "StableCoin (contract)"
participant DjedContract as "Djed (contract)"
StableCoinModule->>ViemFactory: createViemClients(rpcUrl)
ViemFactory-->>StableCoinModule: publicClient, walletClient
StableCoinModule->>ViemPublic: readContract(oracleAddress, OracleABI, "readData")
ViemPublic->>OracleContract: eth_call readData()
OracleContract-->>ViemPublic: oracleData
ViemPublic-->>StableCoinModule: oracleData
StableCoinModule->>ViemPublic: readContract(stableCoinAddress, CoinABI, "totalSupply")
ViemPublic->>StableCoinContract: eth_call totalSupply()
StableCoinContract-->>ViemPublic: totalSupply
ViemPublic-->>StableCoinModule: totalSupply
StableCoinModule->>ViemPublic: readContract(djedAddress, DjedABI, "R", [0])
ViemPublic->>DjedContract: eth_call R(0)
DjedContract-->>ViemPublic: R0
ViemPublic-->>StableCoinModule: R0
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
djed-sdk/src/blockchain/client.js
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/src/blockchain/client.js`:
- Around line 4-8: createViemClients currently passes rpcUrl straight into
http(rpcUrl) which silently falls back to a default when rpcUrl is undefined;
add an explicit guard at the top of createViemClients to validate rpcUrl
(non-null, non-undefined, non-empty string and optionally a valid URL or
startsWith "http") and throw a clear Error (e.g., "createViemClients: invalid
rpcUrl") before calling createPublicClient/createWalletClient so misconfigured
callers fail fast; reference createViemClients and the http(rpcUrl) invocation
when making the change.
- Around line 10-13: The factory currently returns a walletClient created via
createWalletClient with http transport but no hoisted account, which causes
writeContract and other signing actions to fail for HTTP RPC consumers; modify
the factory function that constructs walletClient to accept an optional account
parameter (e.g., account or privateKey) and pass it into createWalletClient so
the returned walletClient is pre-bound to that account, ensuring signing methods
work for functions like writeContract and eliminating the hidden invariant that
callers must supply account per-action.
- Around line 5-13: The factory currently hardcodes chain to mainnet when
calling createPublicClient and createWalletClient; change the factory signature
to accept a chain parameter (defaulting to mainnet) and pass that parameter into
both createPublicClient({ chain }) and createWalletClient({ chain }) so callers
can override for Sepolia/preview/CI while preserving mainnet as the default;
update any references to publicClient and walletClient creation to use the new
parameter name.
- Around line 1-2: The imports createPublicClient, createWalletClient, http
(from "viem") and mainnet (from "viem/chains") will fail because "viem" is not
listed in djed-sdk/package.json; add "viem" under dependencies in
djed-sdk/package.json (not devDependencies), pick a compatible version, then
install/update lockfile so imports in client.js resolve at runtime.
Refactor: Migrate read-only contract calls from web3 to viem publicCl…
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
djed-sdk/src/blockchain/client.js (1)
4-8:⚠️ Potential issue | 🟠 MajorFail fast on invalid
rpcUrlbefore creating clients.
createViemClientscurrently accepts any value and passes it directly tohttp(rpcUrl), which can mask misconfiguration. Add an explicit guard for null/undefined/empty values.🛡️ Suggested fix
export const createViemClients = (rpcUrl) => { + if (typeof rpcUrl !== "string" || rpcUrl.trim() === "") { + throw new Error("createViemClients: invalid rpcUrl"); + } + const publicClient = createPublicClient({ chain: mainnet, transport: http(rpcUrl), })In viem v2, what is the behavior of `http(undefined)` (or empty URL) in `createPublicClient`/`createWalletClient`? Does it fall back to a default chain RPC endpoint?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/blockchain/client.js` around lines 4 - 8, The function createViemClients should fail fast when rpcUrl is null/undefined/empty: add an explicit guard at the start of createViemClients that validates rpcUrl (non-empty string, optionally a valid URL via new URL(...) or a simple truthy/trim check) and throw a descriptive Error (e.g., "createViemClients: rpcUrl is required") before calling http(rpcUrl) or creating publicClient/walletClient; update references to createPublicClient and createWalletClient in this file so they are only invoked after the rpcUrl validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 116-131: The readContract calls in calculateFutureScPrice
reference undeclared identifiers (oracleAddress, OracleABI, stableCoinAddress,
CoinABI, djedAddress, DjedABI) causing ReferenceError; update
calculateFutureScPrice to use the contract instances already passed into the
function (e.g., use the provided oracleContract, stableCoinContract,
djedContract or their equivalent parameter names) and call their read/contract
methods or mapped function names instead of calling publicClient.readContract
with these undefined symbols so the function uses the injected contracts/ABIs
supplied to the module.
---
Duplicate comments:
In `@djed-sdk/src/blockchain/client.js`:
- Around line 4-8: The function createViemClients should fail fast when rpcUrl
is null/undefined/empty: add an explicit guard at the start of createViemClients
that validates rpcUrl (non-empty string, optionally a valid URL via new URL(...)
or a simple truthy/trim check) and throw a descriptive Error (e.g.,
"createViemClients: rpcUrl is required") before calling http(rpcUrl) or creating
publicClient/walletClient; update references to createPublicClient and
createWalletClient in this file so they are only invoked after the rpcUrl
validation.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
djed-sdk/src/djed/stableCoin.js (1)
153-154: Don’t swallow pricing errors in this path.Logging without rethrowing returns
undefined, which can silently propagate invalid UI state. Prefer rethrowing (or returning a structured error contract).Suggested change
} catch (error) { - console.log("calculateFutureScPrice error", error); + console.log("calculateFutureScPrice error", error); + throw error; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/stableCoin.js` around lines 153 - 154, The catch block in calculateFutureScPrice currently only logs errors ("calculateFutureScPrice error", error) which causes the function to return undefined and silently break consumers; update the catch in calculateFutureScPrice to either rethrow the caught error (throw error) or return a structured error result (e.g., { error }) so callers can handle failures; modify the catch in the calculateFutureScPrice function to propagate the error using one of these patterns and ensure any callers handle the thrown/returned error accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 108-116: The change to calculateFutureScPrice now requires
djedAddress/oracleAddress/stableCoinAddress/rpcUrl which breaks existing callers
that pass contract instances; update calculateFutureScPrice to accept either
contract instances or address+rpc inputs: detect whether
djedAddress/oracleAddress/stableCoinAddress are contract objects (e.g., have
expected methods) and use them directly, otherwise treat them as addresses and
instantiate contracts via rpcUrl before proceeding; preserve the original
parameter names (amountBC, amountSC, scDecimalScalingFactor) and behavior so
callers remain compatible, or alternatively introduce a new function name (e.g.,
calculateFutureScPriceFromAddresses) if you intentionally want a separate API.
- Around line 121-136: The calls to publicClient.readContract are passing
artifact objects instead of raw ABI arrays; update the three readContract
invocations that reference OracleABI, CoinABI, and DjedABI to use their .abi
property (e.g., change abi: OracleABI to abi: OracleABI.abi, abi: CoinABI to
abi: CoinABI.abi, and abi: DjedABI to abi: DjedABI.abi) so the readContract
schema validation succeeds (refer to the publicClient.readContract usages around
the readData, totalSupply, and R calls).
---
Nitpick comments:
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 153-154: The catch block in calculateFutureScPrice currently only
logs errors ("calculateFutureScPrice error", error) which causes the function to
return undefined and silently break consumers; update the catch in
calculateFutureScPrice to either rethrow the caught error (throw error) or
return a structured error result (e.g., { error }) so callers can handle
failures; modify the catch in the calculateFutureScPrice function to propagate
the error using one of these patterns and ensure any callers handle the
thrown/returned error accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
djed-sdk/dist/esm/index.jsis excluded by!**/dist/**djed-sdk/dist/umd/index.jsis excluded by!**/dist/**djed-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
djed-sdk/package.jsondjed-sdk/rollup.config.mjsdjed-sdk/src/blockchain/client.jsdjed-sdk/src/djed/stableCoin.js
🚧 Files skipped from review as they are similar to previous changes (1)
- djed-sdk/src/blockchain/client.js
Rav1Chauhan
force-pushed
the
feat/introduce-viem-client
branch
from
73c810a to
3c305e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/package.json`:
- Line 15: The package "viem" is required at runtime by the blockchain client
module (imported in the SDK's blockchain client), but it's currently listed
under devDependencies; move the "viem": "^2.21.53" entry from devDependencies
into dependencies in package.json (preserve the version), update/install to
refresh the lockfile, and verify the published package includes viem so
consumers can resolve the runtime import.
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 124-129: The code references web3Promise inside the stable coin
logic (inside the block using djedContract, oracleContract, stableCoinContract)
but never imports it; add a proper named import for web3Promise at the top of
djed/stableCoin.js from the module that exports it (the same module that defines
web3Promise), ensuring the import name matches the exported symbol and updating
any exports if needed so web3Promise is available at runtime.
- Around line 163-172: The function returns mixed types; normalize to always
return strings: ensure scTargetPrice, the early-return branch (when
futureScSupply === 0n), and the computed futurePrice comparison both return a
string. Update the branches that currently return bigints (including the
viem/publicClient path and the computed futurePrice variable) to convert values
to string before returning so web3Promise() and publicClient.readContract() code
paths (and the comparison between scTargetPrice and futurePrice) produce a
consistent string result; reference symbols: futureScSupply, futureRatio,
scTargetPrice, futurePrice, web3Promise, and publicClient.readContract.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
djed-sdk/dist/esm/index.jsis excluded by!**/dist/**djed-sdk/dist/umd/index.jsis excluded by!**/dist/**djed-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
djed-sdk/package.jsondjed-sdk/rollup.config.mjsdjed-sdk/src/blockchain/client.jsdjed-sdk/src/djed/stableCoin.js
🚧 Files skipped from review as they are similar to previous changes (2)
- djed-sdk/src/blockchain/client.js
- djed-sdk/rollup.config.mjs
djed-sdk/package.json
Outdated
| "author": "", | ||
| "license": "ISC", | ||
| "devDependencies": { | ||
| "viem": "^2.21.53", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== viem imports in source =="
rg -n "from ['\"]viem['\"]|require\(['\"]viem['\"]\)" djed-sdk/src djed-sdk || true
echo "== rollup config: externalization hints =="
rg -n "external|viem" djed-sdk/rollup.config.mjs || true
echo "== package dependency buckets =="
jq '{dependencies, peerDependencies, devDependencies}' djed-sdk/package.json
echo "== built artifacts still referencing viem =="
rg -n "from ['\"]viem['\"]|require\(['\"]viem['\"]\)" djed-sdk/dist || trueRepository: DjedAlliance/StablePay
Length of output: 44642
Move viem to dependencies — it's imported at runtime but missing from consumer scope.
Source code imports viem at runtime (djed-sdk/src/blockchain/client.js), yet it's only listed in devDependencies. Since viem is not marked as external in the rollup config, it is not bundled into the dist output. Consumers will encounter module resolution failures when the SDK code tries to import viem after installation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@djed-sdk/package.json` at line 15, The package "viem" is required at runtime
by the blockchain client module (imported in the SDK's blockchain client), but
it's currently listed under devDependencies; move the "viem": "^2.21.53" entry
from devDependencies into dependencies in package.json (preserve the version),
update/install to refresh the lockfile, and verify the published package
includes viem so consumers can resolve the runtime import.
3c305e8 to
bddf467
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 177-179: The catch block in calculateFutureScPrice currently only
logs errors and returns undefined, which swallows failures; update the catch in
function calculateFutureScPrice to log the error (preserving context) and then
re-throw the caught error so callers can handle failures instead of receiving
undefined.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
djed-sdk/dist/esm/index.jsis excluded by!**/dist/**djed-sdk/dist/umd/index.jsis excluded by!**/dist/**djed-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
djed-sdk/package.jsondjed-sdk/rollup.config.mjsdjed-sdk/src/blockchain/client.jsdjed-sdk/src/djed/stableCoin.js
Rav1Chauhan
force-pushed
the
feat/introduce-viem-client
branch
from
bddf467 to
d30158b
Compare
…cPrice to viem, and update rollup config
Rav1Chauhan
force-pushed
the
feat/introduce-viem-client
branch
from
d30158b to
f6bbfaa
Compare
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
1 participant
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.
Addressed Issues:
Part of migration plan to replace web3.js with viem.
Fixes #79
Screenshots/Recordings:
Not applicable — this PR introduces internal client abstraction and does not modify UI or visible behavior.
Additional Notes:
This PR introduces a viem-based blockchain client abstraction (publicClient and walletClient) without modifying existing contract interaction logic.
No existing functionality is altered.
No breaking changes.
web3.js remains untouched in this PR.
This is the first step in a phased migration from web3.js to viem.
Future PRs will:
Migrate read-only contract calls.
Migrate transaction execution.
Remove web3.js and djed-sdk dependency.
This ensures clean, reviewable incremental migration.
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
New Features
Chores