Revert "feat: update to Evo SDK 3.0.0"#30
Conversation
This reverts commit b83a2b9.
📝 WalkthroughWalkthroughThis pull request updates the SDK API surface by renaming parameters (dataContractId → contractId, documentTypeName → type), removing deprecated platform address APIs, rebranching token operations (emergencyAction → configUpdate), and downgrading dependencies. Corresponding test fixtures, validators, and documentation are updated to align with the new API shapes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/generate_docs.py (1)
90-127: Doc example: use numericstartAtMsinstead of string.
According to Dash Platform SDK documentation,startAtMsis a timestamp in milliseconds (number type), not a string. ChangestartAtMs: '0'tostartAtMs: 0to ensure the example matches the expected parameter type.Proposed change
- startAtMs: '0' + startAtMs: 0
🧹 Nitpick comments (2)
tests/e2e/queries/query-execution.spec.js (2)
50-77: Potential precision loss: “large numbers” parsed viaNumber().
If balances/nonces can exceedNumber.MAX_SAFE_INTEGER, these assertions may silently validate the wrong value. Consider returning abigint(or a string) and asserting on non-negativity/format accordingly.
236-245: Harden validators that assume array payloads.
validateKeysResult,validateTokenBalanceResult, andvalidateTokenInfoResultiterate with.forEachbut don’t assertArray.isArray(...)first; a shape change will throw a confusing runtime error instead of a clear assertion failure.Also applies to: 306-323
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
package.jsonpublic/AI_REFERENCE.mdpublic/api-definitions.jsonpublic/app.jspublic/index.htmlscripts/generate_docs.pytests/e2e/fixtures/test-data.jstests/e2e/queries/query-execution.spec.jstests/e2e/smoke/basic-smoke.spec.jstests/e2e/transitions/state-transitions.spec.jstests/e2e/utils/parameter-injector.jstests/e2e/utils/sdk-page.js
💤 Files with no reviewable changes (1)
- tests/e2e/transitions/state-transitions.spec.js
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-29T14:09:25.873Z
Learnt from: thephez
Repo: dashpay/evo-sdk-website PR: 1
File: tests/e2e/transitions/state-transitions.spec.js:237-378
Timestamp: 2025-09-29T14:09:25.873Z
Learning: Token operations in the Evo SDK (mint, transfer, burn, freeze, destroy frozen, unfreeze, claim, set price, direct purchase, config update) return empty objects {} on success rather than detailed response objects with transaction data.
Applied to files:
tests/e2e/smoke/basic-smoke.spec.js
📚 Learning: 2025-09-29T13:39:22.810Z
Learnt from: thephez
Repo: dashpay/evo-sdk-website PR: 1
File: tests/e2e/smoke/basic-smoke.spec.js:149-163
Timestamp: 2025-09-29T13:39:22.810Z
Learning: The EvoSdkPage class extends BaseTest, which provides the clearResults() method that clicks `#clearButton` and waits for `#identityInfo` to have the 'empty' class. Tests using evoSdkPage.clearResults() are valid due to this inheritance relationship.
Applied to files:
tests/e2e/smoke/basic-smoke.spec.jstests/e2e/queries/query-execution.spec.js
📚 Learning: 2025-10-28T13:24:46.894Z
Learnt from: thephez
Repo: dashpay/evo-sdk-website PR: 16
File: .github/workflows/update-evo-sdk.yml:34-56
Timestamp: 2025-10-28T13:24:46.894Z
Learning: In the dashpay/evo-sdk-website repository, package.json must contain exact version numbers for dashevo/evo-sdk without any version specifiers (no ^, ~, >=, etc.). If version specifiers are present, the workflow should fail.
Applied to files:
package.jsonpublic/index.html
🧬 Code graph analysis (2)
tests/e2e/utils/sdk-page.js (1)
public/app.js (10)
value(1257-1257)value(1263-1263)value(1268-1268)value(1276-1276)value(1297-1297)value(1306-1306)value(1313-1313)value(1476-1476)value(1501-1501)input(1243-1243)
tests/e2e/utils/parameter-injector.js (1)
public/app.js (1)
element(1549-1549)
🪛 Gitleaks (8.30.0)
public/AI_REFERENCE.md
[high] 1064-1064: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1117-1117: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (1)
- GitHub Check: Run SDK Site UI Tests
🔇 Additional comments (28)
package.json (2)
4-4: LGTM - Version downgrade aligns with revert objectives.The exact version
2.1.3complies with the repository requirement for exact version numbers without specifiers. Based on learnings, this format is correct.
25-25: Verify yarn version compatibility.The packageManager is downgraded from
yarn@4.12.0toyarn@4.0.2. Ensure this older yarn version is compatible with the project's lockfile and CI environment.public/index.html (1)
1-187: LGTM - Cosmetic formatting changes only.All changes in this file are whitespace and line-break adjustments to inline attributes and styles. No functional or behavioral changes to the DOM structure, CSP policy, or UI elements.
tests/e2e/utils/parameter-injector.js (1)
1-309: LGTM - Whitespace-only changes.All modifications in this file are blank line adjustments. The parameter injection logic, field mappings, and validation functions remain unchanged.
tests/e2e/utils/sdk-page.js (2)
130-209: LGTM - Simplified parameter filling API.The
fillParameterByNamemethod signature was simplified by removing theisStateTransitionparameter. The method now usesparamNamedirectly for all selector building and lookups, which is cleaner and consistent with the SDK downgrade. The caller at line 604 correctly uses the updated signature.
249-252: Minor: Filter syntax is correct but consider readability.The
.filter({ hasNot: ... })pattern with Playwright locators is correct. This filters out readonly inputs when handling array values.tests/e2e/fixtures/test-data.js (5)
87-93: LGTM - Parameter renamed to match SDK 2.1.3 API.The
requestType→keyRequestTyperename aligns with the older SDK API surface forgetIdentityKeys.
225-245: LGTM - Document query parameters updated.The
documentTypeparameter (previouslydocumentTypeName) is correctly used in document queries, aligning with the SDK 2.1.3 API.
436-484: LGTM - Voting parameters aligned with SDK 2.1.3.Key changes:
contractIdused consistently in voting queriesresultType: "contenders"(wasdocumentsAndVoteTally)allowIncludeLockedAndAbstainingVoteTally(wasincludeLockedAndAbstaining)countinstead oflimitin some contextsThese match the expected API shape for the older SDK version.
612-692: LGTM - Document state transition parameters updated.All document-related state transitions (
documentCreate,documentReplace,documentDelete,documentTransfer,documentPurchase,documentSetPrice) correctly use:
contractId(instead ofdataContractId)documentType(instead ofdocumentTypeName)This is consistent with the SDK 2.1.3 state transition API.
214-222: Parameter naming is intentional and matches API specification.The mixed naming in the test data is correct:
getDataContractHistoryusesdataContractIdwhile other endpoints likegetContestedResourcesusecontractId. The public API definitions confirm each endpoint specifies its own parameter names, so the test data properly reflects the SDK's actual API contract.public/api-definitions.json (4)
49-91: LGTM - Field rename and dependency wiring are consistent.The
keyRequestTyperename (fromrequestType) is correctly applied, and thedependsOn.fieldreferences at lines 74 and 87 properly reference the newkeyRequestTypefield name. ThesearchPurposeMaprename (frompurposeMap) is also consistent.
2151-2232: LGTM - Token config update transition is well-structured.The
tokenConfigUpdatetransition (replacing the previoustokenEmergencyAction) provides a comprehensive set of configuration options with clear labels. The SDK methodtokens.configUpdatealigns with the updated API surface.
1718-1718: SDK method rename looks correct.The
contracts.createmethod name is more intuitive than the previouscontracts.publish.
832-864: LGTM - Vote polls query supports both standard and proof modes.The
getVotePollsByEndDatequery now properly documents the dual-mode time filtering: JSON-basedstartTimeInfo/endTimeInfofor standard responses, and millisecond timestamps (startTimeMs/endTimeMs) when proofs are enabled. The help text clearly indicates which fields to use in each mode.public/AI_REFERENCE.md (4)
1064-1064: Static analysis false positive - example placeholder key in documentation.The Gitleaks warning about a "Generic API Key" at this line is a false positive. This is an intentionally documented example WIF private key (
XFfpaSbZq52HPy3WWwe1dXsZMiU1bQn8vQd34HNXkSZThevBWRn1) used to illustrate SDK usage patterns. The documentation explicitly shows this as a placeholder value in a code example.
85-88: LGTM - Documentation example matches API definition.The
keyRequestTypeparameter in the example correctly matches the renamed field inapi-definitions.json.
1461-1479: LGTM - Token config update documentation is comprehensive.The
tokens.configUpdatetransition documentation correctly lists all availableconfigItemTypeoptions (conventions, max_supply, perpetual_distribution, etc.) matching the options defined inapi-definitions.json.
292-297: The example is correct. The SDK methodcontracts.getHistory()acceptscontractIdas the parameter name (as shown in app.js line 2057). ThedataContractIdname in api-definitions.json is the UI/API layer parameter definition, which is mapped tocontractIdwhen calling the SDK method. No changes needed.tests/e2e/smoke/basic-smoke.spec.js (2)
305-328: Confirm UI label text for “Token Config Update” matches exactly.
The expectation now includes'Token Config Update'; if the UI label is slightly different (e.g., “Token ConfigUpdate” / “Token Config update”), this will fail even if the underlying operation is correct.
361-374: Good: stricter query category assertions viaensureExactOptions.
This should make category drift/duplicates much easier to diagnose than set-based comparisons.tests/e2e/queries/query-execution.spec.js (1)
14-42: Nice stability improvement: wait for proof toggle to actually be checked.
This should reduce flakes where queries run before proof mode is applied.scripts/generate_docs.py (2)
614-629: Output formatting simplification is solid.
JSON.stringify(output, null, 2)is predictable and readable for docs examples.
136-214: Code examples align with@dashevo/evo-sdkv2.1.3 APIs.Verified method signatures:
dpns.usernames(identityId, { limit })— correctly used at line 140tokens.configUpdate(payload)— correctly used at line 373tokens.setPriceForDirectPurchase(payload)— correctly used at line 370Note:
dpns.usernamesWithProof()could not be verified in v2.1.3 documentation and does not appear in the code examples, so no action needed.public/app.js (4)
79-86: Good:tokenConfigUpdateis wired end-to-end (supported transitions + auth requirements).
This matches the rename and should keep the UI consistent with the available transition.Also applies to: 343-346
1966-1988: Good guardrail: disallow proof mode for identity key “search”.
Failing fast here is preferable to returning confusing partial results.
2437-2446: Result formatting looks correct, especially for{}token transition successes.
Given token transitions often return{}on success,JSON.stringifywill produce a clear{}output in the UI. Based on learnings, token operations returning{}is expected.
2054-2059: Remove suggestion: input field forgetDataContractHistoryisdataContractId, notcontractId.The API definitions and test data confirm the documented input field is
dataContractId, notcontractId. The current code correctly usesn.dataContractId || n.idand does not need modification. The proposed fix would add support for a non-existent input field.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Reverts #20 due to issues including #28 and #29.
Edit: test issues resulted at least in part from this commit that effectively neutered many tests. They passed because they were validating very little: b78aa1c#diff-92463ce9e596a6d8eb71e93509be51a9a12cfb043bed5ab845050ea56ef6cedc
Claude evaluation:
The commit systematically weakened almost all validation functions to the point where they no longer catch real issues. Here are the key problems:
Critical Issues:
Summary by CodeRabbit
Release Notes
Chores
Breaking Changes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.