Merged
Conversation
manuelmauro
added
B5-clientnoteworthy
📝 WalkthroughWalkthroughAdds two exported transaction-size constants to test helpers, updates eth-tx size tests to use them (and refines one test's error expectation), updates a README link, and adds "moka" to the license verification script's name list. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
arturgontijo
previously approved these changes
Mar 5, 2026
Contributor
Coverage Report@@ Coverage Diff @@
## master manuel/update-frontier-pin +/- ##
==============================================================
+ Coverage 77.10% 77.15% +0.05%
Files 389 389
+ Lines 76972 77169 +197
==============================================================
+ Hits 59349 59535 +186
+ Misses 17623 17634 +11
|
arturgontijo
dismissed stale reviews from coderabbitai[bot] and themself
via
5ffd73e
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/helpers/constants.ts (1)
19-25: RenameDEFAULT_MAX_TX_INPUT_BYTESto reflect full tx size, not input size.Line 20 documents this as the full signed transaction RLP size, but the symbol name suggests calldata/input bytes. Renaming will reduce future boundary-test confusion.
♻️ Suggested rename (with compatibility alias)
-export const DEFAULT_MAX_TX_INPUT_BYTES = 4 * TX_SLOT_BYTE_SIZE; +export const DEFAULT_MAX_TX_SIZE_BYTES = 4 * TX_SLOT_BYTE_SIZE; +export const DEFAULT_MAX_TX_INPUT_BYTES = DEFAULT_MAX_TX_SIZE_BYTES;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/helpers/constants.ts` around lines 19 - 25, The constant DEFAULT_MAX_TX_INPUT_BYTES is misnamed — it represents the full RLP-encoded signed transaction size, not calldata/input only; rename the symbol to DEFAULT_MAX_TX_SIZE_BYTES (or DEFAULT_MAX_SIGNED_TX_BYTES) and update its JSDoc to match, keep the existing value expression referencing TX_SLOT_BYTE_SIZE, and add a backward-compatible exported alias DEFAULT_MAX_TX_INPUT_BYTES = DEFAULT_MAX_TX_SIZE_BYTES so callers keep working; search and update all internal references/imports to use the new name while leaving the alias for compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/suites/dev/moonbase/test-eth-tx/test-eth-tx-size.ts`:
- Line 18: The test currently constructs data using a hard-coded overhead
subtraction ("FF".repeat(DEFAULT_MAX_TX_INPUT_BYTES - 6474)) which couples the
test to a specific signed/encoded transaction size; change the test to compute
the available payload dynamically by first creating a representative signed
transaction (or using a helper like a new computeSignedTxOverhead function) to
measure its encoded size and then set data to "0x" +
"FF".repeat(DEFAULT_MAX_TX_INPUT_BYTES - measuredOverhead), replacing the magic
number 6474 and using DEFAULT_MAX_TX_INPUT_BYTES and the measuredOverhead
variable so the test remains correct if signing/encoding changes (update the
data assignment and add the measurement helper or inline measurement before
constructing data).
---
Nitpick comments:
In `@test/helpers/constants.ts`:
- Around line 19-25: The constant DEFAULT_MAX_TX_INPUT_BYTES is misnamed — it
represents the full RLP-encoded signed transaction size, not calldata/input
only; rename the symbol to DEFAULT_MAX_TX_SIZE_BYTES (or
DEFAULT_MAX_SIGNED_TX_BYTES) and update its JSDoc to match, keep the existing
value expression referencing TX_SLOT_BYTE_SIZE, and add a backward-compatible
exported alias DEFAULT_MAX_TX_INPUT_BYTES = DEFAULT_MAX_TX_SIZE_BYTES so callers
keep working; search and update all internal references/imports to use the new
name while leaving the alias for compatibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7f8a5a6-4ae9-4246-99bd-80a5e7d6470b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
test/helpers/constants.tstest/suites/dev/moonbase/test-eth-tx/test-eth-tx-size.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify-licenses.sh (1)
48-56:⚠️ Potential issue | 🟠 MajorAvoid crate-name-wide bypass for license verification.
Adding
mokatoNAMES(Line 55) excludes allmokaentries from checks, including future versions. That weakens the compliance guardrail beyond this PR’s immediate dependency state. Prefer a version-scoped exception (and rationale) instead of a global name bypass.Proposed hardening
NAMES=( "webpki" "webpki-roots" "webpki-root-certs" "rustls-webpki" "ring" "nimbus-consensus" - "moka" ) +PACKAGE_EXCEPTIONS=( + "moka@<exact-version>" # temporary; document why and remove when no longer needed +) + +package_filter="" +for pkg in "${PACKAGE_EXCEPTIONS[@]}"; do + name="${pkg%@*}" + version="${pkg#*@}" + package_filter+=" (.name != \"$name\" or .version != \"$version\") and" +done + names_filter=$(printf ' .name != "%s" and' "${NAMES[@]}") # "true" at the end is necessary to close the "and" conditional -filter="${licenses_filter}${authors_filter}${names_filter} true" +filter="${licenses_filter}${authors_filter}${names_filter}${package_filter} true"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/verify-licenses.sh` around lines 48 - 56, The NAMES array currently contains a global exclusion "moka" which bypasses license checks for any moka crate/version; remove the "moka" entry from the NAMES list and replace it with a version-scoped exception mechanism: add a specific allowlist (e.g., ALLOWLIST or VERSIONED_EXCEPTIONS) that stores entries like "moka@<allowed-version>" and update the verification logic that reads NAMES to check for exact name@version matches (or path+version) before skipping license verification; include a brief comment explaining the rationale and record the allowed version and PR/rationale for auditability, and ensure function/loop that originally iterates over NAMES now consults the new versioned allowlist.
🧹 Nitpick comments (1)
pallets/parachain-staking/README.md (1)
19-20: Consolidate duplicated link targets with clearer labeling.Line 19 and Line 20 now point to the same URL but describe it differently (“Unofficial Documentation” vs “(Outdated) Blog Post”). Please keep a single canonical label per URL, or point each label to distinct resources to avoid reader confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pallets/parachain-staking/README.md` around lines 19 - 20, Two markdown links ("Unofficial Documentation" and "(Outdated) Blog Post with Justification") point to the exact same URL; consolidate by keeping a single canonical link label for that URL or ensure each label points to a distinct resource. Edit the README to either remove one of the duplicate lines (keep the preferred label, e.g., "Unofficial Documentation") or update the "(Outdated) Blog Post with Justification" link target to the correct/archive URL; ensure link text clearly reflects the resource (use the exact link texts to locate the entries).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/verify-licenses.sh`:
- Around line 48-56: The NAMES array currently contains a global exclusion
"moka" which bypasses license checks for any moka crate/version; remove the
"moka" entry from the NAMES list and replace it with a version-scoped exception
mechanism: add a specific allowlist (e.g., ALLOWLIST or VERSIONED_EXCEPTIONS)
that stores entries like "moka@<allowed-version>" and update the verification
logic that reads NAMES to check for exact name@version matches (or path+version)
before skipping license verification; include a brief comment explaining the
rationale and record the allowed version and PR/rationale for auditability, and
ensure function/loop that originally iterates over NAMES now consults the new
versioned allowlist.
---
Nitpick comments:
In `@pallets/parachain-staking/README.md`:
- Around line 19-20: Two markdown links ("Unofficial Documentation" and
"(Outdated) Blog Post with Justification") point to the exact same URL;
consolidate by keeping a single canonical link label for that URL or ensure each
label points to a distinct resource. Edit the README to either remove one of the
duplicate lines (keep the preferred label, e.g., "Unofficial Documentation") or
update the "(Outdated) Blog Post with Justification" link target to the
correct/archive URL; ensure link text clearly reflects the resource (use the
exact link texts to locate the entries).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38c8771f-b04c-45b8-9098-fb4eb45c63d7
📒 Files selected for processing (2)
pallets/parachain-staking/README.mdscripts/verify-licenses.sh
librelois
approved these changes
Mar 5, 2026
arturgontijo
added a commit
that referenced
this pull request
Mar 5, 2026
* Update frontier pin to c28328d3 * chore: 📌 properly upgrade pins * chore: 📌 upgrade moonkit * Update Cargo.lock * Fix test D021203 * Fix check-links and licenses --------- Co-authored-by: Artur Gontijo <arturgontijo@gmail.com>
librelois
added
the
D5-nicetohaveaudit⚠️
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
3 participants
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.
Description
Updates the moonbeam-foundation/frontier pin on the moonbeam-polkadot-stable2506 branch from 22da39d0 (moonbeam-foundation/frontier@22da39d) to c28328d3
(moonbeam-foundation/frontier@c28328d).
Included frontier commits
createAndFinalizeBlock()polkadot-evm/frontier#1855 — [Fix] Wait for chain head and eth "latest" in createAndFinalizeBlock()sealing and receipt polling.
eth_blockNumberreturning0x00whenmapping-synclags polkadot-evm/frontier#1832 — Avoid eth_blockNumber returning 0x00 when mapping-sync lags