-
Notifications
You must be signed in to change notification settings - Fork 45
fix(dpp): return UnsupportedFeatureError for legacy state transition structure validation #2891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(dpp): return UnsupportedFeatureError for legacy state transition structure validation #2891
Conversation
…structure validation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
✅ gRPC Query Coverage Report |
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "feadd17bf77ce1af91bbf7f826b9c2b3e580f95e99ae3b7a31d0c885e416341f"
)Xcode manual integration:
|
| fn validate_structure( | ||
| &self, | ||
| platform_version: &PlatformVersion, | ||
| ) -> crate::validation::SimpleConsensusValidationResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
| ) -> crate::validation::SimpleConsensusValidationResult { | |
| ) -> ConsensusValidationResult<bool> { |
where bool:
- true - correct
- false - undecided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outside the normal pattern and confusing. The validation result would pass, but the bool would say it's valid? It's three tiers and hurts my brain.
|
I'm going to merge this in for now, soonish we will move structure validation from drive-abci to dpp. |
Issue being fixed or feature implemented:
Structure validation for legacy/identity-based state transitions was returning an empty success result, which obscured the fact that no actual validation was being performed. This made it unclear
whether validation passed because the transition was valid or because validation wasn't implemented.
What was done:
IdentityUpdate, IdentityCreditTransfer, MasternodeVote) to return UnsupportedFeatureError instead of an empty validation result. This explicitly signals that client-side structure validation is
not implemented for these transition types.
transitions to still be broadcast while address-based transitions get proper structure validation with real error reporting.
The platform still performs full validation during execution, so this change only affects client-side pre-broadcast checks.
Checklist:
For repository code-owners and collaborators only