refactor: introduce adapter layer for Djed transaction handling#98
refactor: introduce adapter layer for Djed transaction handling#98P-ragati wants to merge 5 commits into
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new Djed adapter factory that validates a contract and exposes buy/sell methods; djed.js now uses the adapter and buildTx to create ABI-encoded transaction helper functions for buying and selling stablecoins. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DjedModule
participant Adapter
participant Contract
participant Builder as buildTx
Caller->>DjedModule: request buyScTx(receiver, UI)
DjedModule->>Adapter: createDjedAdapter(djedContract)
Adapter->>Contract: methods.buyStableCoins(receiver, FEE_UI_UNSCALED, UI)
Contract-->>Adapter: method call object (callable)
DjedModule->>DjedModule: .encodeABI() → tx.data
DjedModule->>Builder: buildTx(sender, DJED_ADDRESS, value, data)
Builder-->>Caller: transaction object
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
djed-sdk/src/adapters/djedAdapter.js (1)
4-6: Tighten adapter validation to fail fast on missing contract methods.Line 4 currently validates only
methods; ifbuyStableCoins/sellStableCoinsare missing, failure shifts to a later TypeError. Consider validating required method functions here for a clearer error boundary.Proposed diff
export const createDjedAdapter = (djedContract) => { - if (!djedContract || !djedContract.methods) { - throw new Error("Djed contract instance is required"); + const methods = djedContract?.methods; + if ( + !methods || + typeof methods.buyStableCoins !== "function" || + typeof methods.sellStableCoins !== "function" + ) { + throw new Error("Invalid Djed contract instance: required methods are missing"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/adapters/djedAdapter.js` around lines 4 - 6, The adapter's initial validation only checks djedContract and djedContract.methods but doesn't ensure required RPC methods exist, so add a fail-fast check in the same validation block (where djedContract and djedContract.methods are validated) to assert that djedContract.methods.buyStableCoins and djedContract.methods.sellStableCoins are functions; if any are missing throw a clear Error that names the missing methods (e.g., "Djed contract is missing required methods: buyStableCoins, sellStableCoins") so callers get an immediate, descriptive failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@djed-sdk/src/adapters/djedAdapter.js`:
- Around line 4-6: The adapter's initial validation only checks djedContract and
djedContract.methods but doesn't ensure required RPC methods exist, so add a
fail-fast check in the same validation block (where djedContract and
djedContract.methods are validated) to assert that
djedContract.methods.buyStableCoins and djedContract.methods.sellStableCoins are
functions; if any are missing throw a clear Error that names the missing methods
(e.g., "Djed contract is missing required methods: buyStableCoins,
sellStableCoins") so callers get an immediate, descriptive failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20622e7b-985d-457d-977c-bf0e9bb2479c
📒 Files selected for processing (2)
djed-sdk/src/adapters/djedAdapter.jsdjed-sdk/src/djed/djed.js
…s with clear error messages
There was a problem hiding this comment.
Pull request overview
Refactors Djed contract interaction by introducing a createDjedAdapter layer to centralize transaction-building logic and standardize fee injection.
Changes:
- Added a Djed adapter that validates the contract interface and injects
FEE_UI_UNSCALEDinto buy/sell calls. - Updated Djed transaction-building helpers to use the adapter and a shared
buildTxhelper. - Added
.gitignoreentries for SDK.npmrcandpackage-lock.json.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| djed-sdk/src/djed/djed.js | Uses the new adapter to build buy/sell stablecoin tx calldata and wraps it via buildTx. |
| djed-sdk/src/adapters/djedAdapter.js | Introduces adapter that validates contract methods and injects a standardized fee argument. |
| .gitignore | Adds ignore rules for local npm config and lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Avoid recreating adapter on each call - Rename UI to ui for consistency and clarity
Summary
This PR refactors Djed contract interaction by introducing an adapter layer to centralize transaction-building logic.
Previously, transaction methods like
buyStableCoinsandsellStableCoinswere called directly from multiple locations. This change introduces acreateDjedAdapterabstraction that standardizes how these interactions are handled.Changes Made
Added
djed-sdk/src/adapters/djedAdapter.jsImplemented
createDjedAdapter(djedContract):FEE_UI_UNSCALEDinternallyRefactored:
buyScTxsellScTxRemoved direct dependency on contract method calls in transaction builders
Why this change?
Impact
Notes
Checklist
Summary by CodeRabbit
Refactor
Chores