Fix getbondinfo byte-order mismatch on bond_key - #53
Open
USCMig wants to merge 1 commit into
Open
Conversation
getbondinfo decoded its bond_key hex parameter with a plain, non-reversing hex parse, while every other place a bond key / pubkey travels through the RPC layer (e.g. the unique_public_key and bond_key fields used by wallet_staking_action) goes through PubKeyID's Serialize/Deserialize impls, which byte-reverse for display. The mismatch meant getbondinfo would always look up the wrong 32 bytes for any key produced by the normal staking flow, so it silently returned null for bonds that were genuinely committed on the best chain. Fix by typing the bond_key parameter as PubKeyID instead of a raw String, so it automatically follows the same reversed-hex convention as the rest of the staking RPC surface. Verified live against a real bond: getbondinfo returned null for the hex string reported by wallet_staking_action, but returned correct data (amount, status, last_action_height matching the actual confirmed block) once the same hex string was byte-reversed before decoding.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
getbondinfodecoded itsbond_keyhex parameter with a plain, non-reversing hex parse (Vec::from_hexdirectly into[u8; 32]), while every other place a bond key / pubkey travels through the RPC layer — e.g. theunique_public_key/bond_keyfields used bywallet_staking_action— goes throughPubKeyID'sSerialize/Deserializeimpls, which byte-reverse for display (matching the usual txid/hash display convention). The mismatch meantgetbondinfoalways looked up the wrong 32 bytes for any key produced by the normal staking flow, so it silently returnednull` for bonds that were genuinely active and committed on the best chain.Fix: type the
bond_keyparameter asPubKeyIDinstead of a rawString, so it automatically follows the same reversed-hex convention as the rest of the staking RPC surface (wallet_staking_action'sRetargetDelegationBond,BeginDelegationUnbonding,WithdrawDelegationBondall already usePubKeyIDfor theirbond_keyfields).How this was found
Created a real
CreateNewDelegationBondstaking action viawallet_staking_action, confirmed viagetblock/getblockhashthat the resulting transaction was mined into a canonical, non-reorged block, yetgetbondinforeturnednullfor the exactbond_key(unique_public_key) string returned by that same RPC call. Manually byte-reversing that hex string before queryinggetbondinforeturned the correct bond (amount,status: Active,last_action_heightmatching the confirmed block height) — confirming the byte order, not the underlying bond tracking, was the bug.Test plan
cargo build --release --bin zebradsucceeds cleanly againstmainwith this changegetbondinfolookup for a real, confirmed bond returns correct data once byte order matches thePubKeyIDconvention