Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2250,7 +2250,7 @@ Claim a claimable balance by its balance ID

###### **Options:**

- `--balance-id <BALANCE_ID>` — Balance ID of the claimable balance to claim (64-character hex string)
- `--balance-id <BALANCE_ID>` — Balance ID of the claimable balance to claim. Accepts multiple formats: - API format with type prefix (72 chars): 000000006f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461 - Direct hash format (64 chars): 6f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461 - Address format (base32): BAAMLBZI42AD52HKGIZOU7WFVZM6BPEJCLPL44QU2AT6TY3P57I5QDNYIA

###### **RPC Options:**

Expand Down Expand Up @@ -3193,7 +3193,7 @@ Claim a claimable balance by its balance ID
###### **Options:**

- `--operation-source-account <OPERATION_SOURCE_ACCOUNT>` [alias: `op-source`] — Source account used for the operation
- `--balance-id <BALANCE_ID>` — Balance ID of the claimable balance to claim (64-character hex string)
- `--balance-id <BALANCE_ID>` — Balance ID of the claimable balance to claim. Accepts multiple formats: - API format with type prefix (72 chars): 000000006f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461 - Direct hash format (64 chars): 6f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461 - Address format (base32): BAAMLBZI42AD52HKGIZOU7WFVZM6BPEJCLPL44QU2AT6TY3P57I5QDNYIA

###### **RPC Options:**

Expand Down Expand Up @@ -4809,7 +4809,7 @@ Fetch a claimable balance ledger entry by id

###### **Options:**

- `--id <ID>` — Claimable Balance Ids to fetch an entry for
- `--id <ID>` — Claimable Balance Ids to fetch an entry for. Accepts the 64-char hex hash, the 72-char hex with type prefix returned by Horizon, or the B... address format returned by `getTransaction`
- `--output <OUTPUT>` — Format of the output

Default value: `json`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use soroban_spec_tools::utils::padded_hex_from_str;
#[derive(Parser, Debug, Clone)]
#[group(skip)]
pub struct Cmd {
/// Claimable Balance Ids to fetch an entry for
/// Claimable Balance Ids to fetch an entry for. Accepts the 64-char hex
/// hash, the 72-char hex with type prefix returned by Horizon, or the
/// B... address format returned by `getTransaction`
#[arg(long)]
pub id: Vec<String>,

Expand All @@ -36,11 +38,7 @@ impl Cmd {

fn insert_keys(&self, ledger_keys: &mut Vec<LedgerKey>) -> Result<(), Error> {
for x in &self.id {
let padded_hex = padded_hex_from_str(x, 32)?;
let hash_bytes: [u8; 32] = padded_hex
.try_into()
.map_err(|_| Error::InvalidHash(x.clone()))?;
let hash = Hash(hash_bytes);
let hash = Hash(parse_id(x)?);
let key = LedgerKey::ClaimableBalance(LedgerKeyClaimableBalance {
balance_id: ClaimableBalanceIdTypeV0(hash),
});
Expand All @@ -49,3 +47,60 @@ impl Cmd {
Ok(())
}
}

fn parse_id(x: &str) -> Result<[u8; 32], Error> {
// Accept the same formats as the claimable-balance tx commands (64-char
// hex, 72-char hex with type prefix, B... strkey), falling back to the
// original padded-hex behavior for other input.
if let Ok(bytes) = crate::commands::tx::new::clawback_claimable_balance::parse_balance_id(x) {
return bytes.try_into().map_err(|_| Error::InvalidHash(x.into()));
}
Comment on lines +55 to +57

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair observation - parse_balance_id is a general balance-ID parser that happens to live in the clawback module. Two reasons I kept it here for this focused #2451 fix: (1) claim and clawback are both under tx::new, so that share is local (super::); only the ledger-fetch call crosses trees, and it deliberately discards the tx::args::Error and falls back to padded-hex, so there's no error-type coupling - just one pub fn call. (2) A proper shared util should also carry a neutral error type (it currently returns tx::args::Error, which is wrong outside tx args), and picking the home + error type feels like more than this bugfix should decide on its own. Happy to extract it into commands::tx::utils (or wherever you prefer) - in this PR or a follow-up - if you'd like it in scope.

let padded_hex = padded_hex_from_str(x, 32)?;
padded_hex
.try_into()
.map_err(|_| Error::InvalidHash(x.into()))
}

#[cfg(test)]
mod tests {
use super::*;

const HASH_HEX: &str = "6f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461";

// Regression tests for https://github.com/stellar/stellar-cli/issues/2451:
// accept the same balance-id formats as the claimable-balance tx commands.
#[test]
fn parses_64_char_hex() {
assert_eq!(
parse_id(HASH_HEX).unwrap().to_vec(),
hex::decode(HASH_HEX).unwrap()
);
}

#[test]
fn parses_72_char_hex_with_type_prefix() {
assert_eq!(
parse_id(&format!("00000000{HASH_HEX}")).unwrap().to_vec(),
hex::decode(HASH_HEX).unwrap()
);
}

#[test]
fn parses_strkey() {
let expected = "c58728e6803ee8ea3232ea7ec5ae59e0bc8912debe7214d027e9e36fefd1d80d";
assert_eq!(
parse_id("BAAMLBZI42AD52HKGIZOU7WFVZM6BPEJCLPL44QU2AT6TY3P57I5QDNYIA")
.unwrap()
.to_vec(),
hex::decode(expected).unwrap()
);
}

#[test]
fn short_hex_is_still_padded() {
// Preserves the pre-existing padded-hex behavior of `--id`.
let mut expected = vec![0u8; 31];
expected.push(0xab);
assert_eq!(parse_id("ab").unwrap().to_vec(), expected);
}
}
99 changes: 50 additions & 49 deletions cmd/soroban-cli/src/commands/tx/new/claim_claimable_balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ pub struct Cmd {

#[derive(Debug, clap::Args, Clone)]
pub struct Args {
/// Balance ID of the claimable balance to claim (64-character hex string)
/// Balance ID of the claimable balance to claim. Accepts multiple formats:
/// - API format with type prefix (72 chars): 000000006f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461
/// - Direct hash format (64 chars): 6f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461
/// - Address format (base32): BAAMLBZI42AD52HKGIZOU7WFVZM6BPEJCLPL44QU2AT6TY3P57I5QDNYIA
#[arg(long)]
pub balance_id: String,
}
Expand All @@ -27,75 +30,73 @@ impl TryFrom<&Cmd> for xdr::OperationBody {
op: Args { balance_id },
}: &Cmd,
) -> Result<Self, Self::Error> {
let balance_id_bytes =
hex::decode(balance_id).map_err(|_| tx::args::Error::InvalidHex {
name: "balance-id".to_string(),
hex: balance_id.clone(),
})?;

if balance_id_bytes.len() != 32 {
return Err(tx::args::Error::InvalidHex {
name: "balance-id".to_string(),
hex: balance_id.clone(),
});
}

let mut balance_id_array = [0u8; 32];
balance_id_array.copy_from_slice(&balance_id_bytes);

let claimable_balance_id =
xdr::ClaimableBalanceId::ClaimableBalanceIdTypeV0(xdr::Hash(balance_id_array));

Ok(xdr::OperationBody::ClaimClaimableBalance(
xdr::ClaimClaimableBalanceOp {
balance_id: claimable_balance_id,
balance_id: claimable_balance_id(balance_id)?,
},
))
}
}

fn claimable_balance_id(balance_id: &str) -> Result<xdr::ClaimableBalanceId, tx::args::Error> {
let balance_id_bytes = super::clawback_claimable_balance::parse_balance_id(balance_id)?;

let balance_id_array: [u8; 32] =
balance_id_bytes
.try_into()
.map_err(|_| tx::args::Error::InvalidHex {
name: "balance-id".to_string(),
hex: balance_id.to_string(),
})?;

Ok(xdr::ClaimableBalanceId::ClaimableBalanceIdTypeV0(
xdr::Hash(balance_id_array),
))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_valid_balance_id_hex_parsing() {
let balance_id = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
let balance_id_bytes = hex::decode(balance_id).unwrap();
assert_eq!(balance_id_bytes.len(), 32);
const HASH_HEX: &str = "6f2179b31311fa8064760b48942c8e166702ba0b8fbe7358c4fd570421840461";

let mut balance_id_array = [0u8; 32];
balance_id_array.copy_from_slice(&balance_id_bytes);

let claimable_balance_id =
xdr::ClaimableBalanceId::ClaimableBalanceIdTypeV0(xdr::Hash(balance_id_array));

let op = xdr::ClaimClaimableBalanceOp {
balance_id: claimable_balance_id,
};
fn hash_of(id: &xdr::ClaimableBalanceId) -> Vec<u8> {
let xdr::ClaimableBalanceId::ClaimableBalanceIdTypeV0(hash) = id;
hash.0.to_vec()
}

let xdr::ClaimableBalanceId::ClaimableBalanceIdTypeV0(hash) = op.balance_id;
assert_eq!(hash.0.to_vec(), balance_id_bytes);
#[test]
fn accepts_64_char_hex() {
let id = claimable_balance_id(HASH_HEX).unwrap();
assert_eq!(hash_of(&id), hex::decode(HASH_HEX).unwrap());
}

// Regression tests for https://github.com/stellar/stellar-cli/issues/2451:
// the formats returned by Horizon (72-char hex with type prefix) and by
// `getTransaction` (B... strkey) must be accepted, consistent with
// `clawback-claimable-balance`.
#[test]
fn test_invalid_balance_id_too_short() {
let balance_id = "0123456789abcdef";
let balance_id_bytes = hex::decode(balance_id).unwrap();
assert_ne!(balance_id_bytes.len(), 32);
fn accepts_72_char_hex_with_type_prefix() {
let id = claimable_balance_id(&format!("00000000{HASH_HEX}")).unwrap();
assert_eq!(hash_of(&id), hex::decode(HASH_HEX).unwrap());
}

#[test]
fn test_invalid_balance_id_too_long() {
let balance_id = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef00";
let balance_id_bytes = hex::decode(balance_id).unwrap();
assert_ne!(balance_id_bytes.len(), 32);
fn accepts_strkey() {
let strkey = "BAAMLBZI42AD52HKGIZOU7WFVZM6BPEJCLPL44QU2AT6TY3P57I5QDNYIA";
let expected = "c58728e6803ee8ea3232ea7ec5ae59e0bc8912debe7214d027e9e36fefd1d80d";
let id = claimable_balance_id(strkey).unwrap();
assert_eq!(hash_of(&id), hex::decode(expected).unwrap());
}

#[test]
fn test_invalid_balance_id_not_hex() {
let balance_id = "not_hex_characters_here_not_valid_at_all_exactly_64_chars";
let result = hex::decode(balance_id);
assert!(result.is_err());
fn rejects_invalid_ids() {
// Too short, too long, and non-hex input.
assert!(claimable_balance_id("0123456789abcdef").is_err());
assert!(claimable_balance_id(&format!("{HASH_HEX}00")).is_err());
assert!(
claimable_balance_id("not_hex_characters_here_not_valid_at_all_exactly_64_chars")
.is_err()
);
}
}