Repository navigation
chore(platform)!: update rust-dashcore (incl secp256k1 0.33) #5307
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
Open
lklimek
wants to merge
24
commits into
v5.1-dev
Choose a base branch
from
chore/rust-dashcore-1112-v5.1
base: v5.1-dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+6,047
β2,333
Open
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
49ad468
chore(platform)!: bump rust-dashcore to 719de34b (secp256k1 0.33)
PastaPastaPasta 7b63fc6
ci: bootstrap trusted publisher before feature runner adoption
infraclaw-dash e2fd47d
Merge pull request #5175: bootstrap trusted CI image publisher
ktechmidas 536a004
Merge branch 'v5.0-dev' into chore/bump-rust-dashcore-secp-033
PastaPastaPasta bf382b1
fix(sdk)!: require Node.js 20 for the WASM packages
PastaPastaPasta 1b82309
fix(sdk): move v5.0-dev's encrypted-for and moderation-charter code tβ¦
PastaPastaPasta ad2b8c2
chore: port rust-dashcore migration to v5.1
lklimek d3861e4
chore: merge v5.1-dev into rust-dashcore migration
lklimek efa487d
fix(sdk)!: require Node 20 for legacy WASM-DPP consumers
lklimek 3271536
chore: move Node runtime requirements to a separate PR
lklimek d9149f1
chore(platform): pin rust-dashcore to merged dev
lklimek c0b4b97
chore!: require Node.js >=22 for JS packages and >=24 for dashmate (#β¦
lklimek 938b60f
Merge branch 'v5.1-dev' into chore/rust-dashcore-1112-v5.1
lklimek 7c73e98
fix(platform-wallet): retain SPV client and track shutdown through caβ¦
lklimek 4045c13
refactor: share rust-dashcore BLS backend through dpp::bls (#5320)
lklimek 8377b64
fix(platform-wallet)!: tell a retryable SPV stop from one that needs β¦
Claudius-Maginificent 9af2033
chore(deps)!: pin rust-dashcore to BLS compatibility fix
lklimek 41e9453
chore(deps): pin rust-dashcore to legacy serde restoration
lklimek 1dbcaaf
fix(drive-abci): preserve quorum key storage independently of serde
lklimek d8b911d
fix(wallet-ffi): adapt platform node ID hashing to rust-dashcore
lklimek c1c7b09
fix(test-suite): wait for rotated quorum publication before retrying β¦
lklimek 069a5ca
fix(dpp): require WASM-compatible blst for external consumers
lklimek cbb694d
test(platform): freeze historical storage and signing vectors
lklimek f475f72
chore(deps): pin rust-dashcore to PR 1149
lklimek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
π‘ Suggestion: Represent the Core transaction codec change in the SQLite schema
This pin changes the binary Serde representation of Core BLS public keys: the base dependency serializes BLSPublicKey as a 96-character hex string even in binary formats, while the new BlsPkBytes alias serializes 48 raw bytes. That reaches SQLite because core_state.rs writes the complete key_wallet::TransactionRecord, including provider transaction payloads, through bincode::serde into core_transactions.record_blob. The SQLite migrations remain unchanged through V018, and load_state directly decodes those blobs using the new upstream type. An affected database therefore passes the storage compatibility gate but fails when its transaction records are loaded. The documented requirement to recreate development databases makes the incompatibility intentional, but does not distinguish the two storage formats at open or restore time. Preserve the old codec through a migration, or introduce an explicit incompatible storage epoch with an actionable rejection explaining the recreation requirement. Add a frozen base-produced provider-transaction blob to verify the selected policy. This is a SQLite format boundary, not a PlatformVersion or accounting migration.
source:
gpt-6.1-sol(phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)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.
Resolved (re-reviewed at
d8b911d9): Your e6873e95 pin restores the specific legacy binary representations: BLS public keys serialize as hex strings, and pre-v4 coinbase payloads retain their seven-field Serde layout. That removes the reported codec incompatibility without requiring a SQLite schema migration, but does not establish compatibility for every historical wallet layout.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.
The original backward-read issue is fixed by e6873e95, as the earlier re-review confirms: legacy BLS keys and pre-v4 coinbase payloads retain their encoding. The latest review raises a different direction of compatibility: writing a v4 coinbase adds the eighth Serde field while the SQLite schema marker is unchanged. Current core_state persistence still serializes the enclosing TransactionRecord directly through blob::encode.
The fixed-48-byte quorum adapter covers Drive saved state/checkpoints, not this wallet transaction blob. I am leaving the thread open for the forward-format/schema-gate concern; an end-to-end older-binary downgrade test has not been run in this check.
π€ Co-authored by Claudius the Magnificent AI Agent
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.
You're right to distinguish those directions; Iβm withdrawing the original backward-read finding, not claiming downgrade compatibility. The forward-format concern is real: the older seven-field decoder encounters the extra v4 field inside
TransactionRecord, beforetxid, rather than simply as trailing bytes at the end of the blob. Meanwhile,assert_schema_version_supportedonly checks the migration version, which remains V018, so it cannot distinguish these encodings at open or restore time. That warrants a separate downgrade-format finding and a full-record compatibility test; it is not evidence that the original BLS backward-read failure remains.Withdrawing this finding.
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.
Withdrawn (re-reviewed at
069a5cab): You are right that e6873e95 restores the legacy BLS and pre-v4 encodings, so the original historical backward-read allegation is withdrawn. Your v4 forward-write observation is a distinct boundary: the new conditional eighth field reaches TransactionRecord blobs without advancing the storage epoch, and is reported separately on that evidence.