-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add igra support #260
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
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. Note
|
| Cohort / File(s) | Summary |
|---|---|
EVM Transaction Refactoring components/screens/browser-api/ethereum/send-transaction/SendTransaction.tsx, components/send/evm/erc20-send/ConfirmStep.tsx, components/send/evm/erc721-transfer/Erc721TransferHotWalletConfirm.tsx, components/send/evm/evm-kas-send/ConfirmStep.tsx |
Replaced in-file transaction assembly, nonce/fee handling, and signing with calls to sendEvmTransaction. Removed TransactionSerializable imports and explicit signing/serialization; components now delegate tx construction and broadcast to the helper while preserving state/error handling. |
Transaction Helper Implementation lib/ethereum/transaction.ts |
Added sendEvmTransaction which fetches fee estimates, nonce, and gas price; computes maxFeePerGas/maxPriorityFeePerGas; builds an EIP‑1559 TransactionSerializable, signs via provided signer, broadcasts with ethClient.sendRawTransaction, and returns the tx id. |
Layer2 Configuration lib/layer2.ts |
Added igraTestnet and igraMainnet chain definitions and icons, replaced kasIcon fallback with kaspaIcon, removed the exported EvmChain type, and updated supported EVM L2 chain lists to include IGRA chains. |
Sequence Diagram(s)
sequenceDiagram
participant Component as Component
participant Helper as sendEvmTransaction
participant EthClient as EthClient
participant Signer as Signer
Component->>Helper: sendEvmTransaction(ethClient, signer, sender, to, valueInWei, gas, chainId, data?)
Helper->>EthClient: estimateFeesPerGas()
EthClient-->>Helper: feeEstimates
Helper->>EthClient: getTransactionCount(sender)
EthClient-->>Helper: nonce
Helper->>EthClient: getGasPrice()
EthClient-->>Helper: gasPrice
Helper->>Helper: compute maxFeePerGas & maxPriorityFeePerGas
Helper->>Helper: build TransactionSerializable
Helper->>Signer: signTransaction(transaction)
Signer-->>Helper: signedTx (Hex)
Helper->>EthClient: sendRawTransaction(signedTx)
EthClient-->>Helper: txId
Helper-->>Component: txId
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
🐰 With warren-wide transactions bright,
No duplication in sight!
A helper hops to sign and send,
While IGRA joins the chainly blend,
Sweet code now bounds into the night.
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The title 'feat: add igra support' is partially related to the changeset. While IGRA support is added in lib/layer2.ts, the majority of changes across 4 files focus on refactoring transaction handling to use a new sendEvmTransaction helper function, which is not captured in the title. | Consider updating the title to reflect the primary change, such as 'refactor: centralize EVM transaction sending logic and add IGRA support' to better represent both the core refactoring and the new feature. | |
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/add-igra-network
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds IGRA as a supported EVM L2 network and refactors multiple EVM send flows to use a shared transaction-sending helper.
Changes:
- Add IGRA testnet/mainnet chain configs and include them in supported L2 chain lists.
- Introduce a reusable
sendEvmTransactionhelper to build/sign/broadcast EIP-1559 transactions. - Refactor KAS / ERC-20 / ERC-721 send confirmations and the browser API
eth_sendTransactionpopup to use the helper.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
lib/layer2.ts |
Adds IGRA chain metadata and updates supported chain arrays / default chain icon fallback. |
lib/ethereum/transaction.ts |
New shared helper for EVM tx fee selection, signing, and broadcasting. |
components/send/evm/evm-kas-send/ConfirmStep.tsx |
Uses sendEvmTransaction instead of duplicating tx construction. |
components/send/evm/erc721-transfer/Erc721TransferHotWalletConfirm.tsx |
Uses sendEvmTransaction for ERC-721 transfer submission. |
components/send/evm/erc20-send/ConfirmStep.tsx |
Uses sendEvmTransaction for ERC-20 transfer submission. |
components/screens/browser-api/ethereum/send-transaction/SendTransaction.tsx |
Refactors popup signing flow to use sendEvmTransaction and settings-based chain selection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
components/screens/browser-api/ethereum/send-transaction/SendTransaction.tsx
Show resolved
Hide resolved
components/screens/browser-api/ethereum/send-transaction/SendTransaction.tsx
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Paul Chen <p22626262@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/ethereum/transaction.ts`:
- Around line 34-45: The current logic sets maxPriorityFeePerGas using gasPrice
as a floor which double-counts baseFee; instead compute the tip floor as
gasPrice - baseFee (or derive baseFeePerGas from the latest block) and use that
(or a sensible multiple of estimatedGas.maxPriorityFeePerGas) as the lower bound
for maxPriorityFeePerGas; update the computation of maxPriorityFeePerGas (and
leave maxFeePerGas as the higher of estimatedGas.maxFeePerGas and gasPrice) so
that maxPriorityFeePerGas = max(estimatedGas.maxPriorityFeePerGas, gasPrice -
baseFeePerGas, fallbackTip) using the existing symbols estimatedGas,
maxPriorityFeePerGas, maxFeePerGas, gasPrice and call to
getGasPrice()/provider.getBlock('latest') as needed.
In `@lib/layer2.ts`:
- Around line 52-95: The igraTestnet blockExplorers.default.url has a trailing
slash that is inconsistent with igraTestnet.apiUrl and other explorer URLs;
update the igraTestnet object (specifically blockExplorers.default.url) to
remove the trailing slash (make it
"https://explorer.galleon-testnet.igralabs.com") so it matches the pattern used
by igraMainnet and the apiUrl, and verify no other explorer URL fields contain
unintended trailing slashes.
🧹 Nitpick comments (2)
lib/ethereum/transaction.ts (1)
26-32: Parallelize independent network calls for lower latency.
estimateFeesPerGas,getTransactionCount, andgetGasPriceare independent and can be awaited concurrently. This eliminates two sequential round-trips.Proposed refactor
- const estimatedGas = await ethClient.estimateFeesPerGas(); - const nonce = await ethClient.getTransactionCount({ - address: sender, - }); - - // Get the minimum gas price from the network - const gasPrice = await ethClient.getGasPrice(); + const [estimatedGas, nonce, gasPrice] = await Promise.all([ + ethClient.estimateFeesPerGas(), + ethClient.getTransactionCount({ address: sender }), + ethClient.getGasPrice(), + ]);components/send/evm/erc721-transfer/Erc721TransferHotWalletConfirm.tsx (1)
106-107: Consider logging the error in the catch block.Unlike the other ConfirmStep components (
erc20-send,evm-kas-send) that usecaptureException(e)andconsole.error(e), this catch block silently swallows the error. Adding error reporting would help with debugging failed ERC721 transfers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/ethereum/transaction.ts`:
- Around line 28-30: The nonce-fetch call using ethClient.getTransactionCount({
address: sender }) can collide because it defaults to blockTag: "latest"; change
the call in lib/ethereum/transaction.ts (the ethClient.getTransactionCount usage
that computes the nonce for sender) to request pending nonces by adding
blockTag: "pending" so mempool transactions are counted; if some target chains
don’t support "pending", add a safe fallback (try "pending" and fall back to
"latest" on error) to avoid breaking send logic.
Pull Request Checklist
Before Submission
Description
Additional Notes
By submitting this pull request, I confirm that:
project's
license terms
Summary by CodeRabbit