[Stellar] security contact as contract metadata #679
Conversation
Co-authored-by: Eric Lau <[email protected]>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughReplaces the security contact tag with a generalized contract metadata system across Stellar core, printing, and info-setting. Updates interfaces, builder APIs, and rendering to emit contractmeta!(key="...", val="...") entries. Tests and snapshots are adjusted accordingly. UI tooltip text is updated. Adds a patch changeset for @openzeppelin/wizard-stellar. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as UI (Wizard)
participant Core as setInfo(…)
participant Builder as ContractBuilder
participant Printer as printContract
User->>UI: Enter security contact (email/URL)
UI->>Core: setInfo(builder, { securityContact, license })
Core->>Builder: addContractMetadata({ key: "contact", value: securityContact })
Core->>Builder: license = license (optional)
note over Builder: Builder.metadata updated (Map<key,value>)
UI->>Printer: printContract(contract)
Printer->>Printer: printMetadata(contract.metadata)
Printer-->>UI: contract source with contractmeta!(key="contact", val="...")
note right of Printer: No securityContact tag path\nMetadata drives emission
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/core/stellar/src/set-info.ts (1)
12-16: Consider a more explicit metadata key and trim inputKey 'contact' is generic. 'security-contact' reads clearer and avoids collisions; trimming prevents empty/whitespace-only entries.
Apply this diff:
-export function setInfo(c: ContractBuilder, { securityContact, license }: Info): void { - if (securityContact) c.addContractMetadata({ key: 'contact', value: securityContact }); +export function setInfo(c: ContractBuilder, { securityContact, license }: Info): void { + const sc = securityContact?.trim(); + if (sc) c.addContractMetadata({ key: 'security-contact', value: sc });If you adopt this, update tests/snapshots and any UI copy that shows the key.
packages/core/stellar/src/contract.ts (1)
270-274: Harden addContractMetadata against empty keys/valuesGuard against empty keys and normalize values to reduce surprises.
Apply this diff:
- addContractMetadata(metadata: { key: string; value: string }[] | { key: string; value: string }) { - (Array.isArray(metadata) ? metadata : [metadata]).forEach(({ key, value }) => this.metadata.set(key, value)); - - if (this.metadata.size > 0) this.addUseClause('soroban_sdk', 'contractmeta'); - } + addContractMetadata(metadata: { key: string; value: string }[] | { key: string; value: string }) { + const items = Array.isArray(metadata) ? metadata : [metadata]; + for (const { key, value } of items) { + const k = key?.trim(); + if (!k) continue; + this.metadata.set(k, (value ?? '').toString()); + } + if (this.metadata.size > 0) this.addUseClause('soroban_sdk', 'contractmeta'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/core/stellar/src/contract.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
.changeset/silent-bags-repair.md(1 hunks)packages/core/stellar/src/contract.test.ts(1 hunks)packages/core/stellar/src/contract.test.ts.md(1 hunks)packages/core/stellar/src/contract.ts(3 hunks)packages/core/stellar/src/fungible.compile.test.ts(1 hunks)packages/core/stellar/src/print.ts(2 hunks)packages/core/stellar/src/set-info.ts(1 hunks)packages/ui/src/stellar/InfoSection.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-15T22:49:25.653Z
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: .changeset/sour-hats-grow.md:2-6
Timestamp: 2025-08-15T22:49:25.653Z
Learning: In OpenZeppelin contracts-wizard, breaking changes that have concrete migration paths (like dependency migrations from Community Contracts to OpenZeppelin Contracts) can be handled as minor version bumps instead of major bumps, per maintainer ernestognw's versioning policy.
Applied to files:
.changeset/silent-bags-repair.md
🧬 Code graph analysis (4)
packages/core/stellar/src/contract.test.ts (2)
packages/core/stellar/src/contract.ts (1)
ContractBuilder(74-275)packages/core/stellar/src/print.ts (1)
printContract(20-40)
packages/core/stellar/src/fungible.compile.test.ts (2)
packages/core/stellar/src/utils/compile-test.ts (1)
runRustCompilationTest(40-69)packages/core/stellar/src/fungible.ts (1)
buildFungible(57-88)
packages/core/stellar/src/set-info.ts (1)
packages/core/stellar/src/contract.ts (1)
ContractBuilder(74-275)
packages/core/stellar/src/print.ts (1)
packages/core/stellar/src/contract.ts (1)
Contract(3-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (solidity, default)
- GitHub Check: build (stellar, compile)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
.changeset/silent-bags-repair.md (1)
1-5: Changeset LGTMPatch bump and summary look appropriate for the scope.
packages/core/stellar/src/fungible.compile.test.ts (1)
293-313: Good coverage for metadata pathTest adds a realistic opts.info.securityContact scenario for compilation.
packages/core/stellar/src/print.ts (1)
31-31: Correct placement of metadata emissionPrinting metadata immediately after use clauses is correct (macro is imported before use).
packages/core/stellar/src/contract.test.ts (2)
92-96: LGTMSingle metadata case covered.
98-106: LGTMMulti-metadata and composition with docs covered.
Optionally add a snapshot for escaping to guard against regressions introduced by special chars in metadata. Example test to add:
test('contract metadata escaping', t => { const Foo = new ContractBuilder('Foo'); Foo.addContractMetadata([ { key: 'a"b', value: 'c\\d' }, { key: 'line', value: 'x\ny' }, ]); t.snapshot(printContract(Foo)); });packages/core/stellar/src/contract.test.ts.md (3)
158-173: Snapshot correctnessMacro import precedes contractmeta! and formatting looks consistent.
174-190: Snapshot correctnessMultiple metadata entries emitted in declared order; import present.
191-209: Snapshot correctness with documentationDocs and metadata combine cleanly; ordering is sensible.
packages/core/stellar/src/contract.ts (2)
4-4: Interface extension LGTMContract.metadata added with appropriate type.
77-77: Builder initialization LGTMMap initialized eagerly; fine.
| ␊ | ||
| use soroban_sdk::contractmeta;␊ | ||
| ␊ | ||
| contractmeta!(key="contact", val="[email protected]");␊ |
There was a problem hiding this comment.
I've noticed in the EVM communities it is common to have a regular contact and a security contact. Would it be beneficial to do that in this ecosystem as well? We don't have to add the general contact now, but it might be a good idea to differentiate the security contact now so as to make space for including a general contact in the future.
There was a problem hiding this comment.
Noted, that make sense, security_contact then?
…s-wizard into stellar-security-contact
Fixes #675
Add ability for the Contract model to store metadata
Use this metadata to store security contact