Align crate naming, build all WASM in CI, upgrade SDK, type events - #1222
Merged
yusuftomilola merged 2 commits intoJul 27, 2026
Merged
Conversation
SC-33: crate naming - git mv multisig_transfer -> multisig-transfer so the directory matches the package name. `cargo test -p <dirname>` now works for all five crates. - Normalize the workspace members list: bare paths, alphabetical, no ./ prefix. - Record the convention alongside the members list. SC-34: WASM coverage in CI - build-wasm now builds the whole workspace for wasm32-unknown-unknown rather than only `contrib`. All five crates declare a cdylib crate-type and a #[contract] entrypoint, so all five are deployable and are now verified. - Report per-contract WASM sizes to the job summary and upload the .wasm artifacts. SC-35: soroban-sdk 22 -> 23 - Bumps to 23.5.3, which resolves and passes the suite with no source changes needed for the upgrade itself. - Not 27: it removes ContractEvents::len, breaking the test API, and raises 70 clippy errors. 23 is also the version the sibling ManageHub project runs and is protocol-aligned. Moving further should be its own change. SC-36: standardize event emission - SDK 23 deprecates env.events().publish() in favour of #[contractevent], so all 62 call sites warned and CI runs clippy with -D warnings. This made the two issues one piece of work, which is the order they were sequenced in. - Every event is now a typed #[contractevent] struct in a per-crate events.rs, the only place events are published from. Struct names are <Subject><PastTenseVerb>, the indexable identifier is a #[topic], and the types land in the contract spec so clients can generate bindings instead of decoding positional tuples. - Replaces a mix of symbol_short!, Symbol::new, and bare string-tuple topics in CamelCase and snake_case with one convention. - Adds events to entrypoints that changed state silently, including every initialize, the registrar allowlist changes, the multisig propose/confirm flow, and set_daily_limit. - Splits asset-maintenance's shared create/update paths so an update is distinguishable from a create off-chain. - Publishes the catalogue in contracts/EVENTS.md for the backend bridge. - Adds 13 event-emission tests. The existing contrib event test compared event counts before and after a call. Under SDK 23 env.events().all() reports only the most recent invocation, so that comparison was meaningless; it now asserts the event name and indexed topic. This behaviour is documented in EVENTS.md. Two findings recorded while inventorying events, both out of scope here: contrib's escrow, KYC, staking, oracle, tokenization, detokenization, restrictions and error modules are not declared in lib.rs and so are not compiled (~1,670 lines), and assetsup's register/update/transfer/retire check a caller-supplied address without calling require_auth on it. Closes DistinctCodes#1197 Closes DistinctCodes#1198 Closes DistinctCodes#1199 Closes DistinctCodes#1200
|
@portableDD is attempting to deploy a commit to the naijabuz's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@portableDD Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
yusuftomilola
approved these changes
Jul 27, 2026
yusuftomilola
left a comment
yusuftomilola
left a comment
Collaborator
There was a problem hiding this comment.
Great alignment and modernization! Consistent crate naming, comprehensive WASM CI builds, SDK upgrade, and typed events all improve code quality and developer experience.
yusuftomilola
requested changes
Jul 27, 2026
yusuftomilola
left a comment
yusuftomilola
left a comment
Collaborator
There was a problem hiding this comment.
Hi @portableDD, 👋
This PR has merge conflicts that need to be resolved before it can be merged. The following files have conflicts:
contracts/asset-maintenance/src/lib.rs
contracts/assetsup/src/lib.rs
contracts/assetsup/src/tests/mod.rs
contracts/multisig-transfer/README.md
contracts/multisig-transfer/src/auth_tests.rs
Please resolve these conflicts in your branch and push the updates. Once resolved, the PR can be merged successfully.
Thank you!
…ents Resolves the conflicts with main. The crate rename made this the largest of them: git detected the directory rename and relocated main's new multisig_transfer files (README.md, auth_tests.rs) into multisig-transfer/, which is the right answer and is accepted as-is. assetsup/src/lib.rs: - initialize gained the typed contract_initialized event here and the TTL extend-on-write from SC-44 on main; kept both. - The two-step admin transfer from SC-48 landed on main emitting three events through the deprecated untyped API this branch removes. SDK 23 deprecates env.events().publish(), and CI runs clippy with -D warnings, so leaving them would fail the build. propose_admin, accept_admin and cancel_admin_proposal now emit typed events, with two new #[contractevent] types added for the steps that had no equivalent: AdminProposed and AdminProposalCancelled. Each step is separately observable, which the old single-step admin_changed could not express. asset-maintenance/src/lib.rs: kept main's crate docs with this branch's import line, which no longer needs symbol_short. add_warranty_information gained the require_admin gate from SC-42 on main and the factored store_warranty helper plus typed event here; kept both. assetsup/src/tests/mod.rs: kept both module lists. Two things the merge surfaced that needed real fixes: main does not currently compile its tests. The auth audit (DistinctCodes#1228) added a test calling update_admin, and the two-step admin transfer (DistinctCodes#1229) removed it. The two merged cleanly line by line and broke each other semantically, which no conflict marker catches. Rewritten against the two-step API and split so both halves are covered. This branch's own event test used the same removed entrypoint; it now asserts a distinct event at each step of the transfer, plus cancellation. Documentation the rename made stale is corrected: the crate and workspace READMEs no longer describe the directory/package mismatch as an open problem, and the SECURITY.md line claiming multisig_transfer has no tests is corrected — it has 10 since DistinctCodes#1228, though the request lifecycle itself is still uncovered. All 393 tests pass, clippy and fmt are clean, all five crates resolve by directory name, and all five build to WASM. The 16 remaining untyped publishes are all in contrib's uncompiled files, which is why the build has no deprecation warnings.
yusuftomilola
approved these changes
Jul 27, 2026
yusuftomilola
left a comment
yusuftomilola
left a comment
Collaborator
There was a problem hiding this comment.
Great alignment and modernization! Consistent crate naming, comprehensive WASM CI builds, SDK upgrade, and typed events all improve code quality and developer experience.
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.
Closes #1197
Closes #1198
Closes #1199
Closes #1200