Conversation
This commit updates the dependabot configuration to include additional package ecosystems for documentation generation utilities, published contract packages, and TypeScript SDK packages. Each new entry is scheduled for weekly updates to the development branch, ensuring that dependencies remain current and reducing potential security vulnerabilities. This enhancement streamlines the dependency management process across various project components.
Contributor
📝 WalkthroughWalkthroughAdds Dependabot npm schedules; introduces gas passthrough and broader gas types in the TypeScript SDK; improves timeout cleanup in multiple sanity test scripts; enhances secure-ownable test helpers and pending-transaction discovery/creation; hardens EIP‑712 signing to reject a zero-byte sentinel; adjusts some address/typing in a ganache health check script. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/sanity/guard-controller/base-test.cjs (1)
580-602:⚠️ Potential issue | 🟠 Major
timeoutIdstill leaks on the error path.The timeout is cleared on success, but not when
method.send()fails first.💡 Proposed fix
} catch (error) { + if (timeoutId) clearTimeout(timeoutId); let errorMessage = error.message; // If web3 didn't give revert data for a reverted tx, do a best-effort // eth_call simulation to fetch the revert selector/data for decoding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/sanity/guard-controller/base-test.cjs` around lines 580 - 602, The timeout created for receiptTimeoutMs (stored in timeoutId) is only cleared on the success path; ensure timeoutId is cleared on all exit paths by moving clearTimeout(timeoutId) into a finally block or by calling clearTimeout(timeoutId) at the start of the catch before rethrowing; update the logic around the Promise.race between method.send(...) and the timeoutPromise (symbols: timeoutId, method.send, timeoutPromise, receiptTimeoutMs) so the timer is always cleared to prevent leaks.scripts/sanity/utils/eip712-signing.cjs (1)
118-123:⚠️ Potential issue | 🟠 MajorZERO-sentinel rejection now breaks a current signing path.
createSignedMetaTransaction()still seedsmetaTx.messagewith the zero hash, so this guard can force an immediate throw before signing.💡 Proposed fix
async generateMessageHash(metaTx, contract) { - const hex = this._normalizeMessageHex(metaTx.message); const ZERO_SENTINEL = '0x0000000000000000000000000000000000000000000000000000000000000000'; + const candidate = metaTx?.message === ZERO_SENTINEL + ? (metaTx?.txRecord?.message ?? metaTx?.message) + : metaTx?.message; + const hex = this._normalizeMessageHex(candidate); if (!hex || hex === ZERO_SENTINEL) { throw new Error( 'Meta-transaction missing valid message hash in `metaTx.message`. ' + 'Use generateUnsignedMetaTransactionForNew/Existing so the contract fills this field.' );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/sanity/utils/eip712-signing.cjs` around lines 118 - 123, The current guard in eip712-signing.cjs rejects the ZERO_SENTINEL and breaks the signing flow because createSignedMetaTransaction() intentionally seeds metaTx.message with the zero hash; change the validation so the zero sentinel is allowed during signing: update the check around ZERO_SENTINEL to only throw when hex is null/undefined/empty (e.g., hex == null or hex === '') rather than when it equals ZERO_SENTINEL, so createSignedMetaTransaction(), metaTx.message and downstream signing logic continue to work; run/adjust tests that expect the zero-seeded path to succeed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/typescript/contracts/core/BaseStateMachine.tsx`:
- Around line 91-94: The code forwards options.gas (typed number) directly to
writeContractParams.gas, but viem expects a bigint; update the forwarding in
BaseStateMachine so that when options.gas is defined you convert it to bigint
(e.g., BigInt(options.gas)) before assigning to writeContractParams.gas, and
ensure this mirrors the existing gasPrice conversion logic; reference
TransactionOptions.gas and writeContractParams.gas to locate and change the
assignment, or alternatively change TransactionOptions.gas to bigint (breaking)
if you prefer a type-level fix.
---
Outside diff comments:
In `@scripts/sanity/guard-controller/base-test.cjs`:
- Around line 580-602: The timeout created for receiptTimeoutMs (stored in
timeoutId) is only cleared on the success path; ensure timeoutId is cleared on
all exit paths by moving clearTimeout(timeoutId) into a finally block or by
calling clearTimeout(timeoutId) at the start of the catch before rethrowing;
update the logic around the Promise.race between method.send(...) and the
timeoutPromise (symbols: timeoutId, method.send, timeoutPromise,
receiptTimeoutMs) so the timer is always cleared to prevent leaks.
In `@scripts/sanity/utils/eip712-signing.cjs`:
- Around line 118-123: The current guard in eip712-signing.cjs rejects the
ZERO_SENTINEL and breaks the signing flow because createSignedMetaTransaction()
intentionally seeds metaTx.message with the zero hash; change the validation so
the zero sentinel is allowed during signing: update the check around
ZERO_SENTINEL to only throw when hex is null/undefined/empty (e.g., hex == null
or hex === '') rather than when it equals ZERO_SENTINEL, so
createSignedMetaTransaction(), metaTx.message and downstream signing logic
continue to work; run/adjust tests that expect the zero-seeded path to succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a4341392-06b2-4f9f-93a9-5dafa70e9203
📒 Files selected for processing (8)
.github/dependabot.ymlscripts/sanity-sdk/secure-ownable/base-test.tsscripts/sanity-sdk/secure-ownable/eip712-signing-tests.tsscripts/sanity/guard-controller/base-test.cjsscripts/sanity/runtime-rbac/base-test.cjsscripts/sanity/secure-ownable/base-test.cjsscripts/sanity/utils/eip712-signing.cjssdk/typescript/contracts/core/BaseStateMachine.tsx
This commit enhances the gas limit handling in the BaseStateMachine class by adding validation for the gas option provided by the caller. It ensures that the gas limit is a non-negative integer, converting it to a bigint if necessary. This change improves error handling and provides clearer feedback for invalid gas inputs, aligning with best practices for type safety in TypeScript.
Contributor
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 `@sdk/typescript/contracts/core/BaseStateMachine.tsx`:
- Around line 91-107: The code accepts bigint gas values without ensuring they
are non-negative; after normalizing options.gas to gasLimit (in
BaseStateMachine.tsx where options.gas is handled and before assigning
writeContractParams.gas), add a non-negative check (e.g., if (gasLimit < 0n)
throw new Error(...)) so negative bigint inputs like -1n are rejected with a
clear error message; keep the existing string validation intact and place this
check immediately before writeContractParams.gas = gasLimit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30ee806e-c5bb-4e71-9a89-745c236fc300
📒 Files selected for processing (1)
sdk/typescript/contracts/core/BaseStateMachine.tsx
This commit introduces a check to ensure that the gas limit provided in the BaseStateMachine class is non-negative. If a negative value is detected, an error is thrown with a clear message indicating the issue. This enhancement improves error handling and aligns with best practices for input validation in TypeScript, further strengthening the robustness of the gas limit handling implemented in the previous commit.
Contributor
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 `@sdk/typescript/contracts/core/BaseStateMachine.tsx`:
- Around line 92-105: The runtime accepts bigint and string-like gas values but
TransactionOptions.gas is declared as number and unsafe JS numbers (>
Number.MAX_SAFE_INTEGER) are implicitly allowed; update the TransactionOptions
type to accept number | bigint | string (or a dedicated GasLike union) and
change the validation in BaseStateMachine (where gas parsing occurs) to: accept
bigint directly, accept numeric strings that match /^\d+$/, and reject number
inputs that are not safe integers (Number.isSafeInteger) with a clear error;
ensure conversion to BigInt only happens after these checks so large numeric
strings and bigints work but unsafe JS numbers are rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 121d9d2b-f13e-4ea3-aea7-f41a4e78ace6
📒 Files selected for processing (1)
sdk/typescript/contracts/core/BaseStateMachine.tsx
This commit updates the TransactionOptions interface to allow the gas property to accept a number, bigint, or string. Additionally, it introduces validation in the BaseStateMachine class to ensure that number inputs for gas are non-negative safe integers. This enhancement improves type safety and error handling, providing clearer feedback for invalid gas inputs and aligning with best practices in TypeScript.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/sanity-sdk/secure-ownable/broadcaster-update-tests.ts (1)
95-100: Extract the repeated gas limit into a named constant.The repeated
500_000nvalue is easy to drift over time; centralizing it will keep these flows consistent.♻️ Suggested refactor
+const BROADCASTER_UPDATE_REQUEST_GAS = 500_000n; ... - this.getTxOptions(ownerWallet.address, { gas: 500_000n }) + this.getTxOptions(ownerWallet.address, { gas: BROADCASTER_UPDATE_REQUEST_GAS }) ... - this.getTxOptions(ownerWallet.address, { gas: 500_000n }) + this.getTxOptions(ownerWallet.address, { gas: BROADCASTER_UPDATE_REQUEST_GAS }) ... - this.getTxOptions(ownerWallet.address, { gas: 500_000n }) + this.getTxOptions(ownerWallet.address, { gas: BROADCASTER_UPDATE_REQUEST_GAS }) ... - this.getTxOptions(ownerWallet.address, { gas: 500_000n }) + this.getTxOptions(ownerWallet.address, { gas: BROADCASTER_UPDATE_REQUEST_GAS })Also applies to: 167-171, 220-224, 307-311
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/sanity-sdk/secure-ownable/broadcaster-update-tests.ts` around lines 95 - 100, Multiple calls to secureOwnableOwner.updateBroadcasterRequest (and similar flows) hard-code the gas limit 500_000n; extract this magic number into a single named constant (e.g., DEFAULT_BROADCASTER_GAS = 500_000n) and replace all occurrences (including the calls that pass this value to this.getTxOptions(ownerWallet.address, { gas: 500_000n })) with the constant to ensure consistency across updateBroadcasterRequest flows; declare the constant near the top of the module and use it in each test invocation (references: secureOwnableOwner.updateBroadcasterRequest, this.getTxOptions, ownerWallet.address).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-remote-ganache-health.ts`:
- Line 73: The assignment of accountBloxAddress from dev.AccountBlox.address
(and the similar assignment at line 84) blindly casts strings to `0x${string}`,
allowing malformed addresses to slip through; validate the address before
assigning by using a proper Ethereum address check (e.g., ethers.utils.isAddress
or a /^0x[0-9a-fA-F]{40}$/ regex) and throw or process.exit with a clear error
message including the offending value if validation fails so failures happen
early and clearly (locate the assignments to accountBloxAddress and the other
casted contract address in this file and replace the direct cast with the
validation + fail-fast behavior).
---
Nitpick comments:
In `@scripts/sanity-sdk/secure-ownable/broadcaster-update-tests.ts`:
- Around line 95-100: Multiple calls to
secureOwnableOwner.updateBroadcasterRequest (and similar flows) hard-code the
gas limit 500_000n; extract this magic number into a single named constant
(e.g., DEFAULT_BROADCASTER_GAS = 500_000n) and replace all occurrences
(including the calls that pass this value to
this.getTxOptions(ownerWallet.address, { gas: 500_000n })) with the constant to
ensure consistency across updateBroadcasterRequest flows; declare the constant
near the top of the module and use it in each test invocation (references:
secureOwnableOwner.updateBroadcasterRequest, this.getTxOptions,
ownerWallet.address).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 27293614-1ae2-4297-8ebd-604d8a8a006b
📒 Files selected for processing (6)
scripts/check-remote-ganache-health.tsscripts/sanity-sdk/secure-ownable/broadcaster-update-tests.tsscripts/sanity-sdk/secure-ownable/meta-tx-execution-tests.tsscripts/sanity-sdk/secure-ownable/timelock-period-tests.tssdk/typescript/contracts/core/BaseStateMachine.tsxsdk/typescript/interfaces/base.index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/typescript/contracts/core/BaseStateMachine.tsx
| const dev = deployed.development; | ||
| if (dev && dev.AccountBlox?.address) { | ||
| accountBloxAddress = dev.AccountBlox.address as Address; | ||
| accountBloxAddress = dev.AccountBlox.address as `0x${string}`; |
Contributor
There was a problem hiding this comment.
Validate file/env address strings before assigning contract address.
These direct casts accept malformed values and delay failure until readContract. Add a format check and fail early with a clear message.
🛡️ Suggested fix
let accountBloxAddress: `0x${string}` | null = null;
+const isHexAddress = (v: unknown): v is `0x${string}` =>
+ typeof v === 'string' && /^0x[a-fA-F0-9]{40}$/.test(v);
...
- accountBloxAddress = dev.AccountBlox.address as `0x${string}`;
+ if (isHexAddress(dev.AccountBlox.address)) {
+ accountBloxAddress = dev.AccountBlox.address;
+ } else {
+ console.warn('⚠️ Invalid AccountBlox address in deployed-addresses.json');
+ }
...
- accountBloxAddress = process.env.ACCOUNTBLOX_ADDRESS as `0x${string}`;
+ if (isHexAddress(process.env.ACCOUNTBLOX_ADDRESS)) {
+ accountBloxAddress = process.env.ACCOUNTBLOX_ADDRESS;
+ } else {
+ console.warn('⚠️ Invalid ACCOUNTBLOX_ADDRESS in .env');
+ }Also applies to: 84-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check-remote-ganache-health.ts` at line 73, The assignment of
accountBloxAddress from dev.AccountBlox.address (and the similar assignment at
line 84) blindly casts strings to `0x${string}`, allowing malformed addresses to
slip through; validate the address before assigning by using a proper Ethereum
address check (e.g., ethers.utils.isAddress or a /^0x[0-9a-fA-F]{40}$/ regex)
and throw or process.exit with a clear error message including the offending
value if validation fails so failures happen early and clearly (locate the
assignments to accountBloxAddress and the other casted contract address in this
file and replace the direct cast with the validation + fail-fast behavior).
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.
This commit updates the dependabot configuration to include additional package ecosystems for documentation generation utilities, published contract packages, and TypeScript SDK packages. Each new entry is scheduled for weekly updates to the development branch, ensuring that dependencies remain current and reducing potential security vulnerabilities. This enhancement streamlines the dependency management process across various project components.
Summary by CodeRabbit
Chores
Bug Fixes
New Features
Tests