Skip to content
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.

Sign up for GitHub

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

update sendtoaddress for 0.21 #204

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

update sendtoaddress for 0.21 #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11dc1f8
Add additional options to sendtoaddress as per 0.21.0 docs
luizParreira Sep 28, 2021
0a0c11e
Write test cases for new send_to_address
luizParreira Sep 29, 2021
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
9 changes: 5 additions & 4 deletions client/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ fn handle_defaults<'a, 'b>(
let defaults_i = defaults.len() - 1 - i;
if args[args_i] == serde_json::Value::Null {
if first_non_null_optional_idx.is_some() {
if defaults[defaults_i] == serde_json::Value::Null {
panic!("Missing `default` for argument idx {}", args_i);
}
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason that this is this removed?

Copy link
Author

Choose a reason for hiding this comment

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

Hey, thanks for reviewing the PR :) I removed this so that we can pass null() to the rpc commands and therefore make the node use the default configuration created internally by the node.

Copy link
Member

Choose a reason for hiding this comment

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

I'm only newish to this crate but I'm almost certain we cannot remove this check, doing so changes the behaviour of the whole crate.

args[args_i] = defaults[defaults_i].clone();
}
} else if first_non_null_optional_idx.is_none() {
Expand Down Expand Up @@ -900,6 +897,8 @@ pub trait RpcApi: Sized {
replaceable: Option<bool>,
confirmation_target: Option<u32>,
estimate_mode: Option<json::EstimateMode>,
avoid_reuse: Option<bool>,
fee_rate: Option<i32>,
) -> Result<bitcoin::Txid> {
let mut args = [
address.to_string().into(),
Expand All @@ -910,12 +909,14 @@ pub trait RpcApi: Sized {
opt_into_json(replaceable)?,
opt_into_json(confirmation_target)?,
opt_into_json(estimate_mode)?,
opt_into_json(avoid_reuse)?,
opt_into_json(fee_rate)?,
];
self.call(
"sendtoaddress",
handle_defaults(
&mut args,
&["".into(), "".into(), false.into(), false.into(), 6.into(), null()],
&["".into(), "".into(), false.into(), false.into(), null(), null(), null(), null()],
Comment on lines -918 to +919
Copy link
Member

Choose a reason for hiding this comment

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

I guess 6.into() is the confirmation target, is there a reason that was changed?

Copy link
Author

Choose a reason for hiding this comment

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

According to the docs, the node uses the default value of wallet -txconfirmtarget, this is something each node operator will set on its own, when you are passing 6 as the default, you are overriding that configuration. I am passing null() here so that we don't send any value with the RPC command and the node therefore uses whatever it is configured as default internally to the node, without us having to tell it.

Copy link
Member

Choose a reason for hiding this comment

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

If this is the case this change should probably be done as a separate patch (at the start of the PR) with full justification. This means next time someone is debugging they can use git blame on this line to see the commit that last touched this line and then check that commit to see why we no longer pass in 6.

Sorry for such a delayed response, I was not focused on this crate for along time.

),
)
}
Expand Down
4 changes: 2 additions & 2 deletions integration_test/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ PID1=$!
sleep 3

BLOCKFILTERARG=""
if bitcoind -version | grep -q "v0\.\(19\|2\)"; then
if bitcoind -version | grep -q "v0.\\(19\|20\|21\)"; then
BLOCKFILTERARG="-blockfilterindex=1"
fi

FALLBACKFEEARG=""
if bitcoind -version | grep -q "v0\.2"; then
if bitcoind -version | grep -q "v0.\\(20\|21\\)"; then
FALLBACKFEEARG="-fallbackfee=0.00001000"
fi

Expand Down
39 changes: 21 additions & 18 deletions integration_test/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,17 +393,20 @@ fn test_set_label(cl: &Client) {
fn test_send_to_address(cl: &Client) {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we could skip formatting for this?

#[rustfmt::skip]
fn test_send_to_address(cl: &Client) {
    let addr = cl.get_new_address(None, None).unwrap();
    let est = json::EstimateMode::Conservative;
    let _ = cl.send_to_address(&addr, btc(1), Some("cc"), None, None, None, None, None, None, None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, Some("tt"), None, None, None, None, None, None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, None, Some(true), None, None, None, None, None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, None, None, Some(true), None, None, None, None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, Some(3), None, None, None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, Some(est), None, None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, Some(false), None).unwrap();
    let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, Some(5)).unwrap();
    let _ =cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();
}

let addr = cl.get_new_address(None, None).unwrap();
let est = json::EstimateMode::Conservative;
let _ = cl.send_to_address(&addr, btc(1), Some("cc"), None, None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, Some("tt"), None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, Some(true), None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, Some(true), None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, Some(3), None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, Some(est)).unwrap();
let _ = cl.send_to_address(&addr, btc(1), Some("cc"), None, None, None, None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, Some("tt"), None, None, None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, Some(true), None, None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, Some(true), None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, Some(3), None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, Some(est), None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, Some(false), None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, Some(5)).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();
}

fn test_get_received_by_address(cl: &Client) {
let addr = cl.get_new_address(None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None).unwrap();
let _ = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();
assert_eq!(cl.get_received_by_address(&addr, Some(0)).unwrap(), btc(1));
assert_eq!(cl.get_received_by_address(&addr, Some(1)).unwrap(), btc(0));
let _ = cl.generate_to_address(7, &cl.get_new_address(None, None).unwrap()).unwrap();
Expand All @@ -413,13 +416,13 @@ fn test_get_received_by_address(cl: &Client) {

fn test_list_unspent(cl: &Client) {
let addr = cl.get_new_address(None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();
let unspent = cl.list_unspent(Some(0), None, Some(&[&addr]), None, None).unwrap();
assert_eq!(unspent[0].txid, txid);
assert_eq!(unspent[0].address.as_ref(), Some(&addr));
assert_eq!(unspent[0].amount, btc(1));

let txid = cl.send_to_address(&addr, btc(7), None, None, None, None, None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(7), None, None, None, None, None, None, None, None).unwrap();
let options = json::ListUnspentQueryOptions {
minimum_amount: Some(btc(7)),
maximum_amount: Some(btc(7)),
Expand All @@ -442,7 +445,7 @@ fn test_get_connection_count(cl: &Client) {

fn test_get_raw_transaction(cl: &Client) {
let addr = cl.get_new_address(None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();
let tx = cl.get_raw_transaction(&txid, None).unwrap();
let hex = cl.get_raw_transaction_hex(&txid, None).unwrap();
assert_eq!(tx, deserialize(&Vec::<u8>::from_hex(&hex).unwrap()).unwrap());
Expand All @@ -461,7 +464,7 @@ fn test_get_raw_mempool(cl: &Client) {

fn test_get_transaction(cl: &Client) {
let txid =
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None).unwrap();
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None, None, None).unwrap();
let tx = cl.get_transaction(&txid, None).unwrap();
assert_eq!(tx.amount, sbtc(-1.0));
assert_eq!(tx.info.txid, txid);
Expand All @@ -486,7 +489,7 @@ fn test_list_since_block(cl: &Client) {

fn test_get_tx_out(cl: &Client) {
let txid =
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None).unwrap();
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None, None, None).unwrap();
let out = cl.get_tx_out(&txid, 0, Some(false)).unwrap();
assert!(out.is_none());
let out = cl.get_tx_out(&txid, 0, Some(true)).unwrap();
Expand All @@ -496,17 +499,17 @@ fn test_get_tx_out(cl: &Client) {

fn test_get_tx_out_proof(cl: &Client) {
let txid1 =
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None).unwrap();
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None, None, None).unwrap();
let txid2 =
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None).unwrap();
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None, None, None).unwrap();
let blocks = cl.generate_to_address(7, &cl.get_new_address(None, None).unwrap()).unwrap();
let proof = cl.get_tx_out_proof(&[txid1, txid2], Some(&blocks[0])).unwrap();
assert!(!proof.is_empty());
}

fn test_get_mempool_entry(cl: &Client) {
let txid =
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None).unwrap();
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None, None, None).unwrap();
let entry = cl.get_mempool_entry(&txid).unwrap();
assert!(entry.spent_by.is_empty());

Expand All @@ -516,7 +519,7 @@ fn test_get_mempool_entry(cl: &Client) {

fn test_lock_unspent_unlock_unspent(cl: &Client) {
let addr = cl.get_new_address(None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();

assert!(cl.lock_unspent(&[OutPoint::new(txid, 0)]).unwrap());
assert!(cl.unlock_unspent(&[OutPoint::new(txid, 0)]).unwrap());
Expand Down Expand Up @@ -832,7 +835,7 @@ fn test_finalize_psbt(cl: &Client) {

fn test_list_received_by_address(cl: &Client) {
let addr = cl.get_new_address(None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None).unwrap();
let txid = cl.send_to_address(&addr, btc(1), None, None, None, None, None, None, None, None).unwrap();

let _ = cl.list_received_by_address(Some(&addr), None, None, None).unwrap();
let _ = cl.list_received_by_address(Some(&addr), None, Some(true), None).unwrap();
Expand Down Expand Up @@ -1124,7 +1127,7 @@ fn test_getblocktemplate(cl: &Client) {
// contains an entry in the vector of GetBlockTemplateResultTransaction.
// Otherwise the GetBlockTemplateResultTransaction deserialization wouldn't
// be tested.
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None).unwrap();
cl.send_to_address(&RANDOM_ADDRESS, btc(1), None, None, None, None, None, None, None, None).unwrap();

cl.get_block_template(GetBlockTemplateModes::Template, &[GetBlockTemplateRules::SegWit], &[])
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion json/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ impl<'a> serde::Serialize for ImportMultiRequestScriptPubkey<'a> {
#[derive(Serialize)]
struct Tmp<'a> {
pub address: &'a Address,
}
};
serde::Serialize::serialize(
&Tmp {
address: addr,
Expand Down