Skip to content

feat(statics): enable CoinFeature.STAKING on tmon (SI-2320) - #10131

Merged
damodarnaik699 merged 1 commit into
masterfrom
damodarnaik/si-2320-monadbgjs-add-coinfeaturestaking-to-tmon-and-release-statics
Nov 5, 2026
Merged

damodarnaik699 merged 1 commit into
masterfrom
damodarnaik/si-2320-monadbgjs-add-coinfeaturestaking-to-tmon-and-release-statics

Conversation

@damodarnaik699

@damodarnaik699 damodarnaik699 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add CoinFeature.STAKING to tmon (Testnet Monad) so wallet-platform balance services and bitgo-ui treat Monad testnet as staking-capable.

Context

CoinFeature.STAKING on the coin is what the platform reads to decide whether staking is available for a coin. Without it, staking stays hidden for Monad testnet.

tmon was declared with the shared EVM feature set plus the Monad-specific flags, but no STAKING.

Changes

  • modules/statics/src/allCoinsAndTokens.ts: tmon now also carries CoinFeature.STAKING. Existing features are unchanged.
  • modules/statics/test/unit/base.ts: new Monad staking feature block — asserts tmon exposes STAKING and keeps its existing features, and asserts mon does not expose STAKING.

Mainnet mon is intentionally unchanged (out of scope for SI-2320).

Test

  • yarn unit-test in modules/statics: 41942 passing
  • mocha --grep "Monad staking feature": 2 passing (fails before the source change, passes after)
  • yarn check-fmt and eslint: clean

TICKET: SI-2320

@damodarnaik699
damodarnaik699 requested a review from a team as a code owner November 5, 2026 07:58

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@linear-code

linear-code Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

SI-2320

@damodarnaik699

Copy link
Copy Markdown
Contributor Author

@claude

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Review

The change itself is small and correct: it adds CoinFeature.STAKING to tmon and leaves the other features alone.

Internal information leakage (please fix before merge)

modules/statics/test/unit/base.ts, in the new test block:

  • The comment // mon is gated behind tmon QA — the mainnet rollout lands in a separate ticket describes the rollout process, not what the code does. Suggested rewrite: // mainnet mon does not have STAKING enabled. Alternatively, drop the comment, since the test name already says this.
  • The test name 'mon should not expose STAKING yet' implies a schedule. Suggested rewrite: 'mon should not expose STAKING'.
  • The ticket ID (SI-2320) and the references to wallet-platform internals appear in the PR description. The PR description is also public, so consider trimming "TDD §5.3" and the basePlatformLockBalanceHandler.ts:188 pointer. The commit message footer is fine.

Suggestions

  • The mon negative assertion will fail as soon as mainnet staking is enabled. That forces whoever enables it to edit this test. That may be intended as a guard, but it is a maintenance cost. Check whether other coins in this file do the same thing, and drop it if they don't.
  • The first test mostly re-checks static metadata (fullName, decimals, network) that has nothing to do with staking. Asserting only STAKING would keep the test focused and less brittle.
  • Optional: a describe block per coin is the existing pattern here (e.g. ZAMA). That is consistent, so no change needed.

Questions

  • Does any SDK code path gate behavior on CoinFeature.STAKING (for example, a staking wallet or a coins.forEach filter) that would now start applying to tmon? A quick grep would confirm that nothing unexpected is picked up.

No security or cryptographic concerns.

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

View run

Adds STAKING to tmon (Testnet Monad) so wallet-platform balance services
and bitgo-ui treat Monad testnet as staking-capable. Mainnet mon is left
unchanged pending tmon QA.

TICKET: SI-2320
@damodarnaik699
damodarnaik699 force-pushed the damodarnaik/si-2320-monadbgjs-add-coinfeaturestaking-to-tmon-and-release-statics branch from 696b0c8 to 42fab82 Compare November 5, 2026 08:04
@damodarnaik699

Copy link
Copy Markdown
Contributor Author

@claude review

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @damodarnaik699's task in 19s —— View job


Review

The change is correct and minimal. tmon gains CoinFeature.STAKING (modules/statics/src/allCoinsAndTokens.ts:1865) and its other features are untouched. I didn't run the test suite. I only read the diff.

Earlier automated review

  • The github-actions review above flags a comment (// mon is gated behind tmon QA…) and a test name ('mon should not expose STAKING yet'). Neither is in this diff. The test names are 'tmon should expose STAKING without dropping its existing features' and 'mon should not expose STAKING', and the block has no rollout comment. Nothing needs fixing there.
  • The PR description does include TICKET: SI-2320, which matches the repo's footer convention. I didn't see "TDD §5.3" or basePlatformLockBalanceHandler.ts:188 in the description, so that item looks stale too.

Answer to the open question: SDK code paths that gate on STAKING

  • No module outside modules/statics references CoinFeature.STAKING in src. Adding it to tmon shouldn't change SDK behavior beyond the feature flag that platform services read.
  • In statics tests, the coins.ts hits at lines 76, 443 and 481 are the ones to check. They look like per-coin expected-feature lists. The PR reports 41942 passing in statics, so presumably tmon isn't in them. CI will confirm.

Suggestions (non-blocking)

  • The mon negative assertion matches the existing ZAMA pattern, which also asserts the absence of STAKING (base.ts:420-438). It will break when mainnet staking is enabled, and that is the point of a guard.
  • The SHARED_EVM_SDK check in the first test is a reasonable proxy for "existing features preserved". It's fine as written.

The Node 27-nightly failure is marked experimental and non-blocking. It looks unrelated to this change.

LGTM from a code standpoint.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

@damodarnaik699

Copy link
Copy Markdown
Contributor Author

Addressed in 42fab82:

  • Dropped the rollout-process comment and renamed the test to mon should not expose STAKING.
  • Trimmed the tmon test to the two feature assertions (STAKING present, existing SHARED_EVM_SDK still present); the identity metadata assertions are gone.
  • Trimmed the internal pointers from the PR description.

Kept the mon negative assertion: it pins the split this change makes (testnet staking on, mainnet off), so an accidental mon flip fails loudly instead of shipping silently. It is one line to update when mainnet lands.

On the question about CoinFeature.STAKING consumers: nothing in this repo branches on it. grep -rn "CoinFeature.STAKING" modules/ --exclude-dir=test --exclude-dir=node_modules --exclude-dir=dist matches only modules/statics/src definitions. The consumers are wallet-platform balance services and bitgo-ui, which read the feature off the coin — that is the intent here.

@ashutoshkumar-6 ashutoshkumar-6 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Doddanna17 Doddanna17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@damodarnaik699
damodarnaik699 merged commit bfde884 into master Nov 5, 2026
28 checks passed
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.

3 participants