Skip to content

refactor: const string contract methods#2141

Merged
SimonRastikian merged 11 commits intomainfrom
simon/const-string-contract-methods
Feb 18, 2026
Merged

refactor: const string contract methods#2141
SimonRastikian merged 11 commits intomainfrom
simon/const-string-contract-methods

Conversation

@SimonRastikian
Copy link
Contributor

@SimonRastikian SimonRastikian commented Feb 17, 2026

Closes #2140 and #950

@SimonRastikian SimonRastikian changed the title Simon/const string contract methods refactor:Simon/const string contract methods Feb 17, 2026
@SimonRastikian SimonRastikian changed the title refactor:Simon/const string contract methods refactor: const string contract methods Feb 17, 2026
@SimonRastikian
Copy link
Contributor Author

@DSharifi, I discovered locally that I do not have jq and ruff packages installed while running locally cargo make check-all-fast.
After installing them, the command works so I can debug locally.
Pinging you bcz not sure if we want to add these to nix env too?

@SimonRastikian SimonRastikian marked this pull request as draft February 17, 2026 18:53
@SimonRastikian
Copy link
Contributor Author

SimonRastikian commented Feb 17, 2026

In this PR, I tackled hopefully all the contract methods (including the one where #[private] is mentioned above).
It was mainly generated with Claude, however I reiterated several times over it and read it in a meticulous manner verifying each function name and making sure nothing is left over by mistake (I used regex over the files). However, a couple pair of eyes would not hurt :)

@SimonRastikian SimonRastikian marked this pull request as ready for review February 17, 2026 19:05
@claude
Copy link

claude bot commented Feb 17, 2026

Review: refactor: const string contract methods

Clean refactor - centralizing method name strings into contract_interface::method_names is a good improvement for maintainability.

Minor issue (non-blocking): Two "propose_update" string literals were missed:

  • crates/devnet/src/mpc.rs:479 - still uses "propose_update" instead of method_names::PROPOSE_UPDATE
  • crates/contract/tests/sandbox/common.rs:305 - still uses "propose_update" instead of method_names::PROPOSE_UPDATE

The constant values in method_names.rs are correct and match the actual contract function names. The README fix (start_resharing_instance to start_reshare_instance) correctly reflects the actual method name.

No critical issues found. Approved.

gilcu3
gilcu3 previously approved these changes Feb 17, 2026
Copy link
Contributor

@gilcu3 gilcu3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

My only suggestion is to use the new const directly in crates/node/src/indexer.rs

@SimonRastikian
Copy link
Contributor Author

I went manually through all the functions and added constants. Some are not used but still chose to add the corresponding constant (e.g. VERSION). This is only to allow a better visibility over the API

Copy link
Contributor

@kevindeforth kevindeforth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

@kevindeforth kevindeforth linked an issue Feb 18, 2026 that may be closed by this pull request
@SimonRastikian
Copy link
Contributor Author

@claude review

@SimonRastikian SimonRastikian added this pull request to the merge queue Feb 18, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 18, 2026
@SimonRastikian SimonRastikian added this pull request to the merge queue Feb 18, 2026
@claude
Copy link

claude bot commented Feb 18, 2026

Review: refactor: const string contract methods

No critical issues found. This is a clean mechanical refactor centralizing contract method name strings into contract_interface::method_names. All constant values have been verified to match the actual contract function names. The previously flagged missed propose_update occurrences have been addressed in subsequent commits.

✅ Approved

Merged via the queue into main with commit 44cf6b4 Feb 18, 2026
10 checks passed
@SimonRastikian SimonRastikian deleted the simon/const-string-contract-methods branch February 18, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improving the contract interface Replace hard-coded contract method strings with a typed enum

3 participants