-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add /address/:address/cardinals API #3979
base: master
Are you sure you want to change the base?
Conversation
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.
Looks good so far! Just missing a test
|
||
let mut outputs = index.get_address_info(&address)?; | ||
|
||
outputs.sort(); |
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.
Do people rely on lexicographical ordering? Or should it be ordered by size?
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.
I wasn't sure either! But followed the precedence of address
Line 860 in f1c1ee5
outputs.sort(); |
#[test] | ||
fn address_cardinals_api() { | ||
let core = mockcore::spawn(); | ||
|
||
let ord = TestServer::spawn_with_args( | ||
&core, | ||
&["--index-runes", "--index-sats", "--index-addresses"], | ||
); | ||
|
||
create_wallet(&core, &ord); | ||
core.mine_blocks(1); | ||
|
||
let address = "bc1qxma2dwmutht4vxyl6u395slew5ectfpn35ug9l"; | ||
|
||
let send = CommandBuilder::new(format!("wallet send --fee-rate 13.3 {address} 2btc")) | ||
.core(&core) | ||
.ord(&ord) | ||
.run_and_deserialize_output::<Send>(); | ||
|
||
core.mine_blocks(1); | ||
|
||
let response = ord.json_request(format!("/address/{}/cardinals", address)); | ||
|
||
assert_eq!(response.status(), StatusCode::OK); | ||
|
||
let cardinals_json: Vec<api::Output> = serde_json::from_str(&response.text().unwrap()).unwrap(); | ||
|
||
pretty_assert_eq!( | ||
cardinals_json, | ||
vec![api::Output { | ||
address: Some(address.parse().unwrap()), | ||
inscriptions: vec![], | ||
indexed: true, | ||
runes: BTreeMap::new(), | ||
sat_ranges: Some(vec![(5000000000, 5200000000),]), | ||
script_pubkey: ScriptBuf::from( | ||
address | ||
.parse::<Address<NetworkUnchecked>>() | ||
.unwrap() | ||
.assume_checked() | ||
), | ||
spent: false, | ||
transaction: send.txid, | ||
value: 2 * COIN_VALUE, | ||
}] | ||
); | ||
} | ||
|
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.
Would be good to add a test that asserts that runic and inscription utxos are not returned.
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.
Makes sense, will add them!
Do you think we should have /address/:address/ordinals
endpoint for fetching just inscriptions and runes? I'm not sure about naming it /ordinals
if it includes Runes, but it goes well with /cardinals
.
This would make it easier for apps to rely on the address index for retrieving cardinals UTXOs.