ci: add contract WASM size check and ceiling enforcement (#413) - #439
Open
namdamdoi68-oss wants to merge 1 commit into
Open
ci: add contract WASM size check and ceiling enforcement (#413)#439namdamdoi68-oss wants to merge 1 commit into
namdamdoi68-oss wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a configurable WASM size checker that reports contract sizes, compares them with base artifacts, and enforces a 65,536-byte ceiling in pull request CI. ChangesContract WASM size enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds CI enforcement for contract WASM size limits; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant PullRequestCI
participant BaseWorktree
participant ContractBuild
participant WASMSizeChecker
PullRequestCI->>BaseWorktree: fetch and checkout base commit
BaseWorktree->>ContractBuild: build registry and agents WASM
PullRequestCI->>WASMSizeChecker: pass current and base WASM paths
WASMSizeChecker->>PullRequestCI: report sizes and enforce ceiling
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-wasm-size.sh`:
- Line 46: Require exact contract artifacts throughout the WASM size-check flow.
In scripts/check-wasm-size.sh lines 46-46, reject an empty argument list before
reporting success. In .github/workflows/ci.yml lines 81-81 and 85-85, locate and
copy exactly lodestar_registry.wasm and lodestar_agents.wasm, failing when
either is missing or ambiguous. In .github/workflows/ci.yml lines 93-106,
resolve those exact current-build artifacts, assert both exist, and only then
invoke the checker.
In `@spec.md`:
- Line 22: Standardize the WASM ceiling contract across spec.md:22-22,
scripts/check-wasm-size.sh:6-6, and .github/workflows/ci.yml:90-90. Document and
export the actual variable, MAX_WASM_SIZE_BYTES, with the intended
threshold—either the intentional 64 KiB budget clearly distinguished from the
131,072-byte protocol maximum, or a 131,072-byte default—and update the
check-wasm-size script to read that variable, optionally preserving
WASM_CEILING_BYTES as a compatibility alias.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 417ef2de-1732-4998-9f39-64a36bbf0180
📒 Files selected for processing (5)
.github/workflows/ci.ymlplan.mdscripts/check-wasm-size.shspec.mdtask.md
| - Output formatted WASM sizes and byte deltas compared to the base branch (`main`) on every Pull Request. | ||
|
|
||
| 2. **Ceiling Enforcement**: | ||
| - Enforce a maximum WASM ceiling size (`MAX_WASM_SIZE_BYTES`, default `65536` bytes / 64 KiB per Soroban protocol limit). |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct and standardize the WASM ceiling contract. 65,536 bytes is described as Soroban’s protocol maximum, but Stellar’s current documentation lists a maximum contract Wasm size of 131,072 bytes (128 KiB). Also, the documented MAX_WASM_SIZE_BYTES is never read; the implementation uses WASM_CEILING_BYTES. (developers.stellar.org)
spec.md#L22-L22: document the actual enforced variable and distinguish an intentional 64 KiB budget from the protocol maximum, or change the default to 131,072.scripts/check-wasm-size.sh#L6-L6: read the documented variable (optionally retainingWASM_CEILING_BYTESas a compatibility alias) and use the selected ceiling..github/workflows/ci.yml#L90-L90: export the standardized variable and the intended threshold.
📍 Affects 3 files
spec.md#L22-L22(this comment)scripts/check-wasm-size.sh#L6-L6.github/workflows/ci.yml#L90-L90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec.md` at line 22, Standardize the WASM ceiling contract across
spec.md:22-22, scripts/check-wasm-size.sh:6-6, and
.github/workflows/ci.yml:90-90. Document and export the actual variable,
MAX_WASM_SIZE_BYTES, with the intended threshold—either the intentional 64 KiB
budget clearly distinguished from the 131,072-byte protocol maximum, or a
131,072-byte default—and update the check-wasm-size script to read that
variable, optionally preserving WASM_CEILING_BYTES as a compatibility alias.
namdamdoi68-oss
force-pushed
the
feature/issue-413-wasm-size-check
branch
from
July 29, 2026 08:16
6e1dc4d to
b2821c2
Compare
namdamdoi68-oss
force-pushed
the
feature/issue-413-wasm-size-check
branch
from
July 30, 2026 14:38
b2821c2 to
a07b2ff
Compare
Collaborator
|
Hi @namdamdoi68-oss, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
namdamdoi68-oss
force-pushed
the
feature/issue-413-wasm-size-check
branch
from
July 31, 2026 11:47
a07b2ff to
90a6cd7
Compare
Author
|
Hi! I have resolved the merge conflicts by rebasing onto the latest main branch. All CI steps should now run cleanly. Thanks! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 93-106: Update the WASM artifact discovery in the CI workflow
before the ARGS construction to require exactly one release WASM file per
contract, excluding deps directories, rather than piping find results through
head or silently omitting missing artifacts. Fail the workflow when either
contract has zero or multiple matching files, then pass the validated paths to
check-wasm-size.sh.
- Around line 77-85: Make the base WASM preparation in the CI workflow fail or
explicitly handle unavailable baselines instead of silently continuing: remove
the failure suppression around fetching, worktree creation, contract builds, and
copying; select exactly one release, non-deps WASM for each contract rather than
copying arbitrary matches; and update the ARGS construction to require the
resulting lodestar_registry_base.wasm and lodestar_agents_base.wasm files or
deliberately pass an explicit N/A value.
- Line 77: Update the fetch step containing `git fetch origin` so it no longer
interpolates `github.base_ref` directly in the shell command. Pass the base
reference through the step’s `env` and quote the resulting environment variable,
or use the immutable `github.event.pull_request.base.sha` value instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5fe04a5-8cdc-49c7-85a9-18c5a48b5ba4
📒 Files selected for processing (2)
.github/workflows/ci.ymlscripts/check-wasm-size.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/check-wasm-size.sh
Comment on lines
+77
to
+85
| git fetch origin ${{ github.base_ref }}:base_branch --depth=1 || true | ||
| git worktree add /tmp/base_repo base_branch || true | ||
| if [ -d "/tmp/base_repo/contract" ]; then | ||
| (cd /tmp/base_repo/contract && rustup target add wasm32v1-none && stellar contract build || true) | ||
| find /tmp/base_repo/contract/target -name "*.wasm" -exec cp {} /tmp/base_wasm/lodestar_registry_base.wasm \; 2>/dev/null || true | ||
| fi | ||
| if [ -d "/tmp/base_repo/contract/agents" ]; then | ||
| (cd /tmp/base_repo/contract/agents && rustup target add wasm32v1-none && stellar contract build || true) | ||
| find /tmp/base_repo/contract/agents/target -name "*.wasm" -exec cp {} /tmp/base_wasm/lodestar_agents_base.wasm \; 2>/dev/null || true |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching ci.yml:"
git ls-files | rg '(^|/)\.github/workflows/ci\.yml$' || true
echo
echo "Relevant workflow section:"
if [ -f .github/workflows/ci.yml ]; then
nl -ba .github/workflows/ci.yml | sed -n '60,115p'
fi
echo
echo "Search for base_wasm and ARGS usage:"
rg -n "base_wasm|ARGS|lodestar_registry_base|lodestar_agents_base" .github/workflows/ci.yml . 2>/dev/null | head -200Repository: Stellar-Ecosystem/lodestar
Length of output: 292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Workflow section:"
awk '{printf "%d\t%s\n", NR, $0}' .github/workflows/ci.yml | sed -n '55,115p'
echo
echo "Relevant usages:"
rg -n "base_wasm|ARGS|lodestar_registry_base|lodestar_agents_base" .github/workflows/ci.yml . 2>/dev/null | head -300Repository: Stellar-Ecosystem/lodestar
Length of output: 3874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "check-wasm-size script:"
awk '{printf "%d\t%s\n", NR, $0}' scripts/check-wasm-size.sh | sed -n '1,140p'
echo
echo "Behavioral probe for current workflow ARG construction and artifact copy semantics:"
python3 - <<'PY'
from pathlib import Path
import os
import tempfile
base = Path(tempfile.mkdtemp())
( (base/"contract/target/release") ).mkdir(parents=True)
# Simulate find returning matches in shell glob ordering without -print0 semantics
found = list((base/"contract/target/release").glob("*.wasm"))
print("found_count", len(found))
print("current_workflow_selected_by_head_n_1", (found[0] if found else None))
print("workflow_find_all_would_write", len(found), "files by repeated find -exec cp")
# Simulate missing copy target for current workflow:
base2 = Path(tempfile.mkdtemp())
found2 = list((base2/"contract/target/release").glob("*.wasm"))
print("missing_build_found_count", len(found2))
print("current_workflow_copy_path_exists", (base/base/"lodestar_registry_base.wasm").exists() or False)
print("current_workflow_ARG_contains_missing_base", len(found2) > 0 and "/tmp/base_wasm/lodestar_registry_base.wasm")
PYRepository: Stellar-Ecosystem/lodestar
Length of output: 3733
Do not pass missing or ambiguous base WASM artifacts.
Lines 77-85 ignore base fetch, worktree, build, and copy failures with || true, and find ... -exec cp can overwrite with any matching WASM file. Lines 100-103 then add missing lodestar_registry_base.wasm or lodestar_agents_base.wasm to ARGS. These missing files turn size checks into N/A without failing the PR check. Require one release, non-deps WASM per contract, copy it without || true, and fail or require an explicit N/A path for unavailable baselines.
🧰 Tools
🪛 zizmor (1.28.0)
[error] 77-77: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 77 - 85, Make the base WASM
preparation in the CI workflow fail or explicitly handle unavailable baselines
instead of silently continuing: remove the failure suppression around fetching,
worktree creation, contract builds, and copying; select exactly one release,
non-deps WASM for each contract rather than copying arbitrary matches; and
update the ARGS construction to require the resulting
lodestar_registry_base.wasm and lodestar_agents_base.wasm files or deliberately
pass an explicit N/A value.
namdamdoi68-oss
force-pushed
the
feature/issue-413-wasm-size-check
branch
from
July 31, 2026 12:11
90a6cd7 to
6828d45
Compare
Author
|
Updated WASM artifact discovery in ci.yml with get_single_wasm() validation helper to strictly require exactly 1 release WASM file per contract and fail the CI workflow if 0 or multiple matching files exist. Thanks! |
namdamdoi68-oss
force-pushed
the
feature/issue-413-wasm-size-check
branch
from
July 31, 2026 12:26
6828d45 to
6009262
Compare
Measure compiled Soroban WASM sizes, compute delta against base branch, write GitHub step summaries, and enforce a 64 KiB project budget ceiling (below the 131072-byte protocol maximum). Fix ceiling enforcement and base artifact handling: - validate the ceiling is a positive integer before comparing (invalid values previously made the comparison silently fail and still report Pass) - report N/A instead of a fabricated 0 B delta when the baseline WASM is absent - export MAX_WASM_SIZE_BYTES as the canonical ceiling variable - require exactly one release, non-deps WASM per contract on both PR and base sides Closes Stellar-Ecosystem#413 Signed-off-by: namdamdoi68-oss <namdamdoi68@gmail.com>
namdamdoi68-oss
force-pushed
the
feature/issue-413-wasm-size-check
branch
from
August 14, 2026 10:44
c8e0e10 to
bd694e7
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
2 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.
Adds a WASM binary size check script and CI step that measures compiled contract sizes, computes delta against main, prints a summary table, and fails if any contract exceeds the 64 KiB ceiling.
Closes #413
Summary by CodeRabbit