fix(stellar): assert on raw token name in generated rust tests#824
fix(stellar): assert on raw token name in generated rust tests#824Assassin859 wants to merge 2 commits into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe ChangesRust name test token name fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/stellar/src/zip-shared.test.ts (1)
203-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGood coverage for spaces; consider adding a quote-character case.
This test correctly validates that test('printRustNameTest uses raw token name with spaces in assertion', t => { the raw name flows into the assertion while identifiers stay sanitized. Given the escaping concern raised on
printRustNameTest/printVaultRustTestinzip-shared.ts, consider adding a case with a token name containing a"character once that fix lands, to lock in the escaping behavior.🤖 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 `@packages/core/stellar/src/zip-shared.test.ts` around lines 203 - 231, Add coverage for the escaping path in printRustNameTest by extending the existing assertion-style test to use a token name containing a double quote character, not just spaces. Keep the contract identifier sanitized while verifying the raw token name is safely escaped in the generated Rust assertion string. Use printRustNameTest and the related printVaultRustTest escaping behavior as the reference points when updating the test expectations.
🤖 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 `@packages/core/stellar/src/zip-shared.ts`:
- Line 19: The generated Rust test strings in printRustNameTest and
printVaultRustTest are interpolating tokenName directly into String::from_str
literals, which can break the emitted test.rs when tokenName contains quotes or
a trailing backslash. Update the shared Rust string generation in zip-shared.ts
to escape tokenName for Rust string literals before inserting it into both
assertions, using the existing printRustNameTest and printVaultRustTest paths.
---
Nitpick comments:
In `@packages/core/stellar/src/zip-shared.test.ts`:
- Around line 203-231: Add coverage for the escaping path in printRustNameTest
by extending the existing assertion-style test to use a token name containing a
double quote character, not just spaces. Keep the contract identifier sanitized
while verifying the raw token name is safely escaped in the generated Rust
assertion string. Use printRustNameTest and the related printVaultRustTest
escaping behavior as the reference points when updating the test expectations.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d3a3e801-852e-420f-b9ee-d078add9d8a2
📒 Files selected for processing (3)
packages/core/stellar/src/zip-rust.tspackages/core/stellar/src/zip-shared.test.tspackages/core/stellar/src/zip-shared.ts
| } | ||
|
|
||
| export const printRustNameTest = (c: Pick<Contract, 'constructorArgs' | 'name'>) => `#![cfg(test)] | ||
| export const printRustNameTest = (c: Pick<Contract, 'constructorArgs' | 'name'>, tokenName: string) => `#![cfg(test)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline packages/core/stellar/src/zip-shared.ts --view expanded || true
echo
echo "== relevant lines =="
sed -n '1,220p' packages/core/stellar/src/zip-shared.ts | cat -n
echo
echo "== search for tokenName interpolation and any escaping helpers =="
rg -n "tokenName|escapeRust|String::from_str|printRustNameTest|printVaultRustTest" packages/core/stellar/src/zip-shared.ts packages/core/stellar/src -SRepository: OpenZeppelin/contracts-wizard
Length of output: 29087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tokenName origin =="
sed -n '1,140p' packages/core/stellar/src/zip-rust.ts | cat -n
echo
echo "== tests around special characters =="
sed -n '190,245p' packages/core/stellar/src/zip-shared.test.ts | cat -n
echo
echo "== search for any escaping/encoding before printRustNameTest / printVaultRustTest =="
rg -n "escapeRust|replace\\(/\\\\/g|replace\\(/\"/g|tokenName.*replace|opts\\.name|name:.*tokenName|printRustNameTest\\(|printVaultRustTest\\(" packages/core/stellar/src -SRepository: OpenZeppelin/contracts-wizard
Length of output: 5485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== name validation / schema for GenericOptions =="
sed -n '1,220p' packages/core/stellar/src/build-generic.ts | cat -n
echo
echo "== ContractBuilder / opts.name sources in Stellar code =="
sed -n '1,180p' packages/core/stellar/src/contract.ts | cat -n
sed -n '1,140p' packages/core/stellar/src/fungible.ts | cat -n
sed -n '1,120p' packages/core/stellar/src/stablecoin.ts | cat -n
sed -n '1,120p' packages/core/stellar/src/non-fungible.ts | cat -n
sed -n '1,120p' packages/core/stellar/src/vault.ts | cat -n
echo
echo "== search for name sanitization / allowed-character checks =="
rg -n "name.*regex|regex.*name|allowed.*name|sanitize.*name|validate.*name|opts\\.name|name\\s*:" packages/core/stellar/src -SRepository: OpenZeppelin/contracts-wizard
Length of output: 50385
Escape tokenName before embedding it in Rust strings. Both printRustNameTest and printVaultRustTest interpolate tokenName directly into String::from_str(&env, "..."); a " or trailing \ will break the generated test.rs. Escape it for Rust string literals before writing both assertions.
🤖 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 `@packages/core/stellar/src/zip-shared.ts` at line 19, The generated Rust test
strings in printRustNameTest and printVaultRustTest are interpolating tokenName
directly into String::from_str literals, which can break the emitted test.rs
when tokenName contains quotes or a trailing backslash. Update the shared Rust
string generation in zip-shared.ts to escape tokenName for Rust string literals
before inserting it into both assertions, using the existing printRustNameTest
and printVaultRustTest paths.
1 participant
Description
This PR fixes a bug in the generated Soroban Rust tests where the name assertion fails for token names containing spaces or special characters.
Root Cause
ContractBuilderconstructor converts a user-supplied name (e.g."My Token") to a valid Rust struct identifier (e.g."MyToken") and stores it asc.name.printRustNameTestandprintVaultRustTest) were usingc.name(the Rust struct identifier).client.name()returns the raw token name supplied toBase::set_metadata(...)(e.g."My Token").Changes
printRustNameTestandprintVaultRustTestinsidezip-shared.tsto accepttokenName: stringand use it in the assertion rather thanc.name.opts.name(the raw user input) into the test printing functions withinzip-rust.ts.