feat(rs-sdk): expose transition hash from state transition methods #3092
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| use std::ops::Deref; | ||
|
Collaborator
Author
There was a problem hiding this comment. Good question. Yes, it would be cleaner to include the transition hash directly in the result types ( The wrapper approach (
But Pasta's right that embedding it in T would be simpler for consumers — no I'd lean toward keeping the wrapper for now since it's non-invasive, but happy to refactor if you prefer the embedded approach. What's your preference? |
||
|
|
||
| /// Wrapper that bundles a state transition proof result with the transition hash. | ||
| /// | ||
| /// The transition hash (also known as the transaction ID) is computed deterministically | ||
| /// from the serialized `StateTransition` before broadcast, so it does not depend on | ||
| /// blockchain state and there is no race condition. | ||
| /// | ||
| /// `StateTransitionResult<T>` implements `Deref<Target = T>`, so existing code that | ||
| /// only needs the inner result can use it transparently. | ||
| #[derive(Debug, Clone)] | ||
|
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: find . -type f -name "state_transition_result.rs" | head -5Repository: dashpay/platform Length of output: 128 🏁 Script executed: cat -n packages/rs-sdk/src/platform/transition/state_transition_result.rs | head -50Repository: dashpay/platform Length of output: 2044 🏁 Script executed: git log --oneline --all | grep -i "22fc89c1c\|state.transition.result" | head -10Repository: dashpay/platform Length of output: 144 🏁 Script executed: git show --stat 22fc89c1c 2>/dev/null | head -30Repository: dashpay/platform Length of output: 42 🏁 Script executed: git log --all --oneline --decorate | grep -i "state.transition" | head -20Repository: dashpay/platform Length of output: 1741 🏁 Script executed: git log --all --grep="PartialEq\|Eq" --oneline | head -10Repository: dashpay/platform Length of output: 42 🏁 Script executed: rg "StateTransitionResult" packages/rs-sdk/src/platform/transition/ -A 5 -B 2Repository: dashpay/platform Length of output: 10366 🏁 Script executed: git log --all --oneline packages/rs-sdk/src/platform/transition/state_transition_result.rs | head -10Repository: dashpay/platform Length of output: 130 🏁 Script executed: git show d6f4eb9ac --stat | head -40Repository: dashpay/platform Length of output: 2283 🏁 Script executed: git log --all --oneline --grep="22fc89c1c" | head -5Repository: dashpay/platform Length of output: 42 🏁 Script executed: git log --all --oneline | grep -E "22fc89c1c|3077" | head -5Repository: dashpay/platform Length of output: 135 🏁 Script executed: git show d6f4eb9ac:packages/rs-sdk/src/platform/transition/state_transition_result.rs 2>/dev/null | head -20Repository: dashpay/platform Length of output: 42 🏁 Script executed: git show d6f4eb9ac --format=fuller | head -100Repository: dashpay/platform Length of output: 4611 🏁 Script executed: git log --all --oneline -S "PartialEq" -- packages/rs-sdk/src/platform/transition/state_transition_result.rs | head -10Repository: dashpay/platform Length of output: 42 🏁 Script executed: git log --all --oneline -S "Eq" -- packages/rs-sdk/src/platform/transition/state_transition_result.rs | head -10Repository: dashpay/platform Length of output: 42 🏁 Script executed: git show d6f4eb9ac -- packages/rs-sdk/src/platform/transition/state_transition_result.rs | head -50Repository: dashpay/platform Length of output: 42 🏁 Script executed: git show d6f4eb9ac:packages/rs-sdk/src/platform/transition/state_transition_result.rs 2>&1 | catRepository: dashpay/platform Length of output: 179 🏁 Script executed: git log --all --pretty=format:"%H %s" | grep "3077"Repository: dashpay/platform Length of output: 408 🏁 Script executed: git show --name-status d6f4eb9ac | grep "state_transition_result"Repository: dashpay/platform Length of output: 42 🏁 Script executed: git diff d6f4eb9ac~1 d6f4eb9ac -- packages/rs-sdk/src/platform/transition/state_transition_result.rsRepository: dashpay/platform Length of output: 42 🏁 Script executed: git log --all --oneline -S "StateTransitionResult" -- "**.rs" | head -20Repository: dashpay/platform Length of output: 146 🏁 Script executed: git log --oneline --all packages/rs-sdk/src/platform/transition/ | head -20Repository: dashpay/platform Length of output: 1669 🏁 Script executed: git show HEAD:packages/rs-sdk/src/platform/transition/state_transition_result.rs 2>&1 | head -30Repository: dashpay/platform Length of output: 1067 🏁 Script executed: rg "PartialEq.*Eq" packages/rs-sdk/src/platform/transition/ -B 2Repository: dashpay/platform Length of output: 42 🏁 Script executed: git show 07b2e5cba --format=fuller | head -50Repository: dashpay/platform Length of output: 2109 🏁 Script executed: git show 07b2e5cba -- packages/rs-sdk/src/platform/transition/state_transition_result.rs | head -80Repository: dashpay/platform Length of output: 2982 🏁 Script executed: git log --all -p -- packages/rs-sdk/src/platform/transition/state_transition_result.rs | grep -A 3 -B 3 "PartialEq\|Eq" | head -40Repository: dashpay/platform Length of output: 42 🏁 Script executed: git show 07b2e5cba --stat | grep "state_transition_result"Repository: dashpay/platform Length of output: 245 🏁 Script executed: git log --all --oneline --grep="post-review\|22fc89c1c" | head -10Repository: dashpay/platform Length of output: 42 🏁 Script executed: rg "StateTransitionResult" packages/rs-sdk/src/platform/transition/broadcast.rs -B 2 -A 2 | head -50Repository: dashpay/platform Length of output: 1363 Add The struct only derives 🛠️ Proposed fix-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
pub struct StateTransitionResult<T> {🤖 Prompt for AI Agents |
||
| pub struct StateTransitionResult<T> { | ||
|
Comment on lines
+3
to
+12
Member
There was a problem hiding this comment. claw; would it make more sense for the T to just include the transition hash? is that possible? |
||
| inner: T, | ||
| transition_hash: [u8; 32], | ||
|
Collaborator
Author
There was a problem hiding this comment. 🟡 Suggestion: Use the existing TxId newtype instead of raw [u8; 32] for transition_hash The source: ['claude'] 🤖 Fix this with AI agents |
||
| } | ||
|
|
||
| impl<T> StateTransitionResult<T> { | ||
| /// Creates a new result bundling the proof result with the transition hash. | ||
| pub fn new(inner: T, transition_hash: [u8; 32]) -> Self { | ||
| Self { | ||
| inner, | ||
| transition_hash, | ||
| } | ||
| } | ||
|
|
||
| /// Returns the transition hash (transaction ID) as a 32-byte array. | ||
| pub fn transition_hash(&self) -> [u8; 32] { | ||
| self.transition_hash | ||
| } | ||
|
|
||
| /// Consumes this wrapper, returning the inner result and the transition hash. | ||
| pub fn into_parts(self) -> (T, [u8; 32]) { | ||
| (self.inner, self.transition_hash) | ||
| } | ||
|
|
||
| /// Consumes this wrapper, returning just the inner result. | ||
| pub fn into_inner(self) -> T { | ||
| self.inner | ||
| } | ||
|
|
||
| /// Maps the inner value, preserving the transition hash. | ||
| pub fn map<U, F: FnOnce(T) -> U>(self, f: F) -> StateTransitionResult<U> { | ||
| StateTransitionResult { | ||
| inner: f(self.inner), | ||
| transition_hash: self.transition_hash, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<T> Deref for StateTransitionResult<T> { | ||
| type Target = T; | ||
|
|
||
| fn deref(&self) -> &T { | ||
| &self.inner | ||
| } | ||
| } | ||
|
Comment on lines
+1
to
+56
Collaborator
Author
There was a problem hiding this comment. 🟡 Suggestion: No unit tests for StateTransitionResult The new type has 6 public methods ( source: ['claude', 'codex'] 🤖 Fix this with AI agents
Comment on lines
+50
to
+56
Collaborator
Author
There was a problem hiding this comment. 💬 Nitpick: Deref impl on a non-smart-pointer type is unconventional The Rust API guidelines recommend source: ['claude'] |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,6 +96,6 @@ impl WasmSdk { | |
| .await | ||
| .map_err(|e| WasmSdkError::generic(format!("Failed to broadcast: {}", e)))?; | ||
|
|
||
| convert_proof_result(result.into_inner()).map_err(WasmSdkError::from) | ||
|
Collaborator
Author
There was a problem hiding this comment. 🟡 Suggestion: WASM broadcastAndWait discards the transition hash before crossing the FFI boundary The WASM SDK's public If this is intentionally deferred to the #2953 follow-up, consider adding a brief source: ['claude', 'codex'] 🤖 Fix this with AI agents |
||
| } | ||
| } | ||
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.
💬 Nitpick: transaction_id() serializes the state transition multiple times in the broadcast_and_wait flow
In
broadcast_and_wait,self.transaction_id()?is called at line 274. Thenself.broadcast()callsself.transaction_id()again insidetrace!()at line 42-44, andself.wait_for_response()does the same at line 101-102. When trace-level logging is enabled, this results in 3 serializations of the state transition. This is negligible in practice since trace logging is rarely active in production, but the hash could be passed into the inner methods to avoid the redundancy.source: ['claude']