Skip to content

Commit 8601bf9

Browse files
LeoNeroLeoNero
authored
Unit tests for models.rs and lib.rs; integration tests for clients (#65)
* docs(README): improve `Testing` section * feat(models): derive `PartialEq` for some types in src/models.rs `PartialEq` was derived for the following types: - BlockTemplate - Transaction - SubaddressBalanceData - BalanceData - TransferPriority - SubaddressData - SubaddressIndex - Payment - AddressData - IncomingTransfers - GotAccount - GetAccountsData - GotTransfer - SignedKeyImage - KeyImageImportResponse * feat(models): add GenerateBlocksResponseR struct * feat(models): add GenerateBlocksResponse struct * feat(models): add `From<GenerateBlocksResponseR>` for `GenerateBlocksResponse` * fix(models): fix type of TransferData's `tx_key` field * doc(models): add comment about possible optional field Comment is about the possibility of GenerateFromKeysArgs's `password` field being optional, even though Monero docs do not mention it. * test(models): test conversion from GenerateBlocksResponseR to GenerateBlocksResponse * docs(lib): fix small typos and improve wording a little * fix(lib): fix parameter name passed to `get_balance` RPC call * refactor(lib): remove unncessary full type path from `json_params` in `daemon_rpc_call` method * feat(lib): add `all` parameter for `export_key_images` RPC call * docs(lib): add comment about get_bulk_payments's parameter min_block_height possibly being optional * feat(lib): `generate_blocks` method now returns `GenerateBlocksResponse` * fix(lib): fix `submit_block` method and return type * fix(lib): handle `on_get_block_hash` inconsistency on invalid height * fix(lib): fix `get_payments` to make it return a vector of payments * fix(lib): fix type of `check_tx_key` RPC call parameters and return type * test(lib): add unit tests for RpcParams * test(lib): add serialization test for TransferType * test(lib): add serialization and deserialization test for TransferPriority * test(rpc): remove old tests in tests/rpc.rs * test(rpc): add helpers functions to test DaemonRpcClient * test(rpc): add helpers functions to test RegtestDaemonJsonRpcClient * test(rpc): add helpers functions to test WalletClient * test(rpc): add `common::helpers` module * test(rpc): add `basic_wallet` test * test(rpc): add `empty_blockchain` test * test(rpc): add `non_empty_blockchain` test * test(rpc): add `basic_daemon_rpc` test * test(rpc): add tests of interaction between all clients * test(rpc): add `common::main_tests` module * test(rpc): add `common` crate * test(rpc): add code to run all tests in `common::main_tests` * test(rpc): fix the tests * test(rpc): bump to Monero v0.18.0.0 * docs(CHANGELOG): update CHANGELOG to reflect recent changes * refactor(tests): remove `common` folder * refactor(tests): allow a lint on `empty_blockchain`.rs and add explanation * refactor(tests): remove `async` declaration from function that don't need it * fix(lib): fix Rust 1.56.1 format error in on_get_block_hash * docs(rpc): improve test documentation * test(rpc): rename main_tests to clients_tests, and `fn test` to `fn run` * refactor(tests/wallet): fix expected_ naming inconsistency * refactor(rpc): make tests functions more explicit * docs(rpc): fix typo in a comment on rpc.rs
1 parent e9683fe commit 8601bf9

16 files changed

Lines changed: 3032 additions & 308 deletions

‎CHANGELOG.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Add tests for types implementing `HashType` in `utils.rs` ([#59](https://github.com/monero-ecosystem/monero-rpc-rs/pull/59))
1313
- Add tests for `HashString`'s implementation of the traits `Debug`, `Serialize`, and `Deserialize`, in `utils.rs` ([#59](https://github.com/monero-ecosystem/monero-rpc-rs/pull/59))
1414
- Add tests for `models.rs` ([#63](https://github.com/monero-ecosystem/monero-rpc-rs/pull/63))
15+
- Add `PartialEq` trait for the following types in `src/models.rs` ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/)):
16+
- BlockTemplate
17+
- Transaction
18+
- SubaddressBalanceData
19+
- BalanceData
20+
- TransferPriority
21+
- SubaddressData
22+
- SubaddressIndex
23+
- Payment
24+
- AddressData
25+
- IncomingTransfers
26+
- GotAccount
27+
- GetAccountsData
28+
- GotTransfer
29+
- SignedKeyImage
30+
- KeyImageImportResponse
31+
- Add `GenerateBlocksResponse` struct ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
32+
- Add `all` paremeter, of type `Option<bool>` to the `export_key_images` method, and pass it to the RPC ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
33+
- Add an error for `on_get_block_hash` on invalid height, instead of returning success with an incorrect hash ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
1534

1635
### Removed
1736

@@ -29,6 +48,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2948
- Change `get_address_index` to return `anyhow::Result<subaddress::Index>` instead of `anyhow::Result<(u64, u64)>` ([#62](https://github.com/monero-ecosystem/monero-rpc-rs/pull/62))
3049
- Use `Amount` type from `monero-rs` where possible ([#68](https://github.com/monero-ecosystem/monero-rpc-rs/pull/68))
3150
- Rename `DaemonClient` to `DaemonJsonRpcClient`, and `RegtestDaemonClient` to `RegtestDaemonJsonRpcClient` ([70](https://github.com/monero-ecosystem/monero-rpc-rs/pull/70))
51+
- Change `TransferData`'s `tx_key` field from `HashString<CryptoNoteHash>` to `HashString<Vec<u8>>` ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
52+
- `get_balance` method now passes the correct parameter name to the RPC ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
53+
- Change `generate_blocks` to return `anyhow::Result<GenerateBlocksResponse>` instead of `anyhow::Result<u64>` ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
54+
- `submit_block` method now works correctly and had its return type changed to `anyhow::Result<()>` ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
55+
- Change `get_payments` to actually return a vector of `Payments` ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
56+
- Change `check_tx_key`'s `tx_key` parameter to type `Vec<u8>` ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
57+
- Change `check_tx_key`'s return type from `anyhow::Result<(NonZeroU64, bool, NonZeroU64)>` to `anyhow::Result<(u64, bool, Amount)>`, since the first element can be `0`, and the last element depicts an amount ([#65](https://github.com/monero-ecosystem/monero-rpc-rs/pull/65/))
3258

3359
## [0.1.0] - 2022-06-29
3460

‎README.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ use monero_rpc::{RpcClient, JsonTransaction};
1919
#[tokio::test]
2020
async fn monero_daemon_transactions_test() {
2121
let tx_id = "7c50844eced8ab78a8f26a126fbc1f731134e0ae3e6f9ba0f205f98c1426ff60".to_string();
22-
let daemon_client = RpcClient::new("http://node.monerooutreach.org:18081".to_string());
23-
let daemon = daemon_client.daemon_rpc();
22+
let rpc_client = monero_rpc::RpcClient::new("http://node.monerooutreach.org:18081".to_string());
23+
let daemon_rpc_client = rpc_client.daemon_rpc();
2424
let mut fixed_hash: [u8; 32] = [0; 32];
2525
hex::decode_to_slice(tx_id, &mut fixed_hash).unwrap();
26-
let tx = daemon
26+
let tx = daemon_rpc_client
2727
.get_transactions(vec![fixed_hash.into()], Some(true), Some(true))
2828
.await;
2929
println!("tx {:?}", tx);
@@ -36,14 +36,16 @@ async fn monero_daemon_transactions_test() {
3636

3737
## Testing
3838

39-
First, you'll need `docker` and `docker-compose` to run the RPC integration tests in case you don't want to run `monerod` and `monero-wallet-rpc` on your own.
39+
First, you'll need `docker` and `docker-compose` to run the RPC integration tests, which are in `tests/`, in case you don't want to run `monerod` and `monero-wallet-rpc` on your own.
4040

4141
If you have the docker stack installed, go to the `tests` folder and run `docker-compose up`. Note that the daemon will run on port `18081` and `monero-wallet-rpc` will run on port `18083`.
4242

4343
After that, just run `cargo test` as you normally would.
4444

4545
Also, you can run `docker-compose down` to stop and remove the two containers started by `docker-compose up`.
4646

47+
**Important**: the blockchain must be empty when running the `main_functional_test` test on `tests/rpc.rs`, i.e. it must have only the genesis block. In `regtest`, the blockchain restarts when `monerod` restarts (as a side note, if you want to keep the blockchain in `regtest` between restarts, you should pass the `--keep-fakechain` flag when starting `monerod`).
48+
4749
## Releases and Changelog
4850

4951
See [CHANGELOG.md](CHANGELOG.md) and [RELEASING.md](RELEASING.md).

‎src/lib.rs‎

Lines changed: 155 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
//! ## Usage
1818
//!
1919
//! Create the base [`RpcClient`] and use the methods [`RpcClient::daemon`],
20-
//! [`RpcClient::daemon_rpc`], or [`RpcClient::wallet`] to retreive the specialized RPC client.
20+
//! [`RpcClient::daemon_rpc`], or [`RpcClient::wallet`] to retrieve the specialized RPC client.
2121
//!
2222
//! On a [`DaemonJsonRpcClient`] you can call [`DaemonJsonRpcClient::regtest`] to get a [`RegtestDaemonJsonRpcClient`]
23-
//! instance that enable RPC call specific to regtest such as
23+
//! instance that enables RPC call specific to regtest such as
2424
//! [`RegtestDaemonJsonRpcClient::generate_blocks`].
2525
//!
2626
//! ```rust
@@ -44,8 +44,8 @@ pub use self::{models::*, util::*};
4444
use jsonrpc_core::types::{Id, *};
4545
use monero::{
4646
cryptonote::{hash::Hash as CryptoNoteHash, subaddress},
47-
util::address::PaymentId,
48-
Address,
47+
util::{address::PaymentId, amount},
48+
Address, Amount,
4949
};
5050
use serde::{de::IgnoredAny, Deserialize, Deserializer, Serialize, Serializer};
5151
use serde_json::{json, Value};
@@ -137,7 +137,7 @@ impl RemoteCaller {
137137
let client = self.http_client.clone();
138138
let uri = format!("{}/{}", &self.addr, method);
139139

140-
let json_params: jsonrpc_core::types::params::Params = params.into();
140+
let json_params: Params = params.into();
141141

142142
trace!(
143143
"Sending daemon RPC call: {:?}, with params {:?}",
@@ -203,7 +203,7 @@ impl RpcClient {
203203
}
204204

205205
/// Transform the client into the specialized `DaemonJsonRpcClient` that interacts with JSON RPC
206-
/// Methods on daemon.
206+
/// methods on daemon.
207207
pub fn daemon(self) -> DaemonJsonRpcClient {
208208
let Self { inner } = self;
209209
DaemonJsonRpcClient { inner }
@@ -281,13 +281,24 @@ impl DaemonJsonRpcClient {
281281

282282
/// Look up a block's hash by its height.
283283
pub async fn on_get_block_hash(&self, height: u64) -> anyhow::Result<BlockHash> {
284-
self.inner
284+
let res = self
285+
.inner
285286
.request::<HashString<BlockHash>>(
286287
"on_get_block_hash",
287288
RpcParams::array(once(height.into())),
288289
)
289290
.await
290-
.map(|v| v.0)
291+
.map(|v| v.0)?;
292+
293+
// see https://github.com/monero-ecosystem/monero-rpc-rs/issues/58 for rationality
294+
if res == BlockHash::from_slice(&[0; 32]) {
295+
Err(anyhow::Error::msg(format!(
296+
"Invalid height {} supplied.",
297+
height
298+
)))
299+
} else {
300+
Ok(res)
301+
}
291302
}
292303

293304
/// Get a block template on which mining a new block.
@@ -314,13 +325,15 @@ impl DaemonJsonRpcClient {
314325
}
315326

316327
/// Submit a mined block to the network.
317-
pub async fn submit_block(&self, block_blob_data: String) -> anyhow::Result<String> {
328+
pub async fn submit_block(&self, block_blob_data: String) -> anyhow::Result<()> {
318329
self.inner
319-
.request(
330+
.request::<IgnoredAny>(
320331
"submit_block",
321332
RpcParams::array(once(block_blob_data.into())),
322333
)
323-
.await
334+
.await?;
335+
336+
Ok(())
324337
}
325338

326339
/// Retrieve block header information matching selected filter.
@@ -436,12 +449,7 @@ impl RegtestDaemonJsonRpcClient {
436449
&self,
437450
amount_of_blocks: u64,
438451
wallet_address: Address,
439-
) -> anyhow::Result<u64> {
440-
#[derive(Deserialize)]
441-
struct Rsp {
442-
height: u64,
443-
}
444-
452+
) -> anyhow::Result<GenerateBlocksResponse> {
445453
let params = empty()
446454
.chain(once((
447455
"amount_of_blocks",
@@ -454,10 +462,13 @@ impl RegtestDaemonJsonRpcClient {
454462

455463
Ok(self
456464
.inner
457-
.request::<MoneroResult<Rsp>>("generateblocks", RpcParams::map(params))
465+
.request::<MoneroResult<GenerateBlocksResponseR>>(
466+
"generateblocks",
467+
RpcParams::map(params),
468+
)
458469
.await?
459470
.into_inner()
460-
.height)
471+
.into())
461472
}
462473
}
463474

@@ -599,7 +610,7 @@ impl WalletClient {
599610
.chain(once(("account_index", account_index.into())))
600611
.chain(address_indices.map(|v| {
601612
(
602-
"adress_indices",
613+
"address_indices",
603614
v.into_iter().map(Value::from).collect::<Vec<_>>().into(),
604615
)
605616
}));
@@ -709,14 +720,21 @@ impl WalletClient {
709720

710721
/// Get a list of incoming payments using a given payment id.
711722
pub async fn get_payments(&self, payment_id: PaymentId) -> anyhow::Result<Vec<Payment>> {
723+
#[derive(Deserialize)]
724+
struct Rsp {
725+
#[serde(default)]
726+
payments: Vec<Payment>,
727+
}
728+
712729
let params = empty().chain(once((
713730
"payment_id",
714731
HashString(payment_id).to_string().into(),
715732
)));
716733

717734
self.inner
718-
.request("get_payments", RpcParams::map(params))
735+
.request::<Rsp>("get_payments", RpcParams::map(params))
719736
.await
737+
.map(|rsp| rsp.payments)
720738
}
721739

722740
/// Get a list of incoming payments using a given payment id, or a list of payments ids, from a
@@ -726,6 +744,7 @@ impl WalletClient {
726744
pub async fn get_bulk_payments(
727745
&self,
728746
payment_ids: Vec<PaymentId>,
747+
// It seems that the `min_block_height` argument is really optional, but the docs on the Monero website do not mention it
729748
min_block_height: u64,
730749
) -> anyhow::Result<Vec<Payment>> {
731750
#[derive(Deserialize)]
@@ -989,7 +1008,7 @@ impl WalletClient {
9891008
.await
9901009
}
9911010

992-
/// Show information about a transfer to/from this address. **Called `get_transfer_by_txid` in
1011+
/// Show information about a transfer to/from this address. **Calls `get_transfer_by_txid` in
9931012
/// RPC.**
9941013
pub async fn get_transfer(
9951014
&self,
@@ -1025,7 +1044,10 @@ impl WalletClient {
10251044
}
10261045

10271046
/// Export a signed set of key images.
1028-
pub async fn export_key_images(&self) -> anyhow::Result<Vec<SignedKeyImage>> {
1047+
pub async fn export_key_images(
1048+
&self,
1049+
all: Option<bool>,
1050+
) -> anyhow::Result<Vec<SignedKeyImage>> {
10291051
#[derive(Deserialize)]
10301052
struct R {
10311053
key_image: HashString<Vec<u8>>,
@@ -1055,8 +1077,10 @@ impl WalletClient {
10551077
}
10561078
}
10571079

1080+
let params = empty().chain(all.map(|v| ("all", v.into())));
1081+
10581082
self.inner
1059-
.request::<Rsp>("export_key_images", RpcParams::None)
1083+
.request::<Rsp>("export_key_images", RpcParams::map(params))
10601084
.await
10611085
.map(From::from)
10621086
}
@@ -1094,14 +1118,15 @@ impl WalletClient {
10941118
pub async fn check_tx_key(
10951119
&self,
10961120
txid: CryptoNoteHash,
1097-
tx_key: CryptoNoteHash,
1121+
tx_key: Vec<u8>,
10981122
address: Address,
1099-
) -> anyhow::Result<(NonZeroU64, bool, NonZeroU64)> {
1123+
) -> anyhow::Result<(u64, bool, Amount)> {
11001124
#[derive(Deserialize)]
11011125
struct Rsp {
1102-
confirmations: NonZeroU64,
1126+
confirmations: u64,
11031127
in_pool: bool,
1104-
received: NonZeroU64,
1128+
#[serde(with = "amount::serde::as_pico")]
1129+
received: Amount,
11051130
}
11061131

11071132
let params = empty()
@@ -1136,3 +1161,105 @@ impl WalletClient {
11361161
Ok((u16::try_from(major)?, u16::try_from(minor)?))
11371162
}
11381163
}
1164+
1165+
#[cfg(test)]
1166+
mod tests {
1167+
use super::*;
1168+
use serde_test::{assert_de_tokens_error, assert_ser_tokens, assert_tokens, Token};
1169+
1170+
#[test]
1171+
fn rpc_params_array() {
1172+
let mut array = once(json!(false));
1173+
let rpc_params_array = RpcParams::array(array.clone());
1174+
1175+
if let RpcParams::Array(mut rpc_boxed_array) = rpc_params_array {
1176+
assert_eq!(rpc_boxed_array.next(), array.next());
1177+
assert_eq!(rpc_boxed_array.next(), None);
1178+
assert_eq!(array.next(), None);
1179+
} else {
1180+
unreachable!();
1181+
}
1182+
}
1183+
1184+
#[test]
1185+
fn rpc_params_map() {
1186+
let map = once(("it is false", json!(false)));
1187+
let rpc_params_map = RpcParams::map(map.clone());
1188+
1189+
let mut map = map.map(|(k, v)| (k.to_string(), v));
1190+
1191+
if let RpcParams::Map(mut boxed_map) = rpc_params_map {
1192+
assert_eq!(boxed_map.next(), map.next());
1193+
assert_eq!(boxed_map.next(), None);
1194+
assert_eq!(map.next(), None);
1195+
} else {
1196+
unreachable!();
1197+
}
1198+
}
1199+
1200+
#[test]
1201+
fn from_rpc_params_for_params() {
1202+
let rpc_param_array = RpcParams::array(once(json!(false)));
1203+
let rpc_param_map = RpcParams::map(once(("it is false", json!(false))));
1204+
let rpc_param_none = RpcParams::None;
1205+
1206+
assert_eq!(Params::from(rpc_param_none), Params::None);
1207+
assert_eq!(
1208+
Params::from(rpc_param_array),
1209+
Params::Array(vec![json!(false)])
1210+
);
1211+
1212+
let mut serde_json_map = serde_json::map::Map::new();
1213+
serde_json_map.insert("it is false".to_string(), json!(false));
1214+
1215+
assert_eq!(Params::from(rpc_param_map), Params::Map(serde_json_map));
1216+
}
1217+
1218+
#[test]
1219+
fn serialize_transfer_type() {
1220+
let transfer_types = vec![
1221+
TransferType::All,
1222+
TransferType::Available,
1223+
TransferType::Unavailable,
1224+
];
1225+
assert_ser_tokens(
1226+
&transfer_types,
1227+
&[
1228+
Token::Seq { len: Some(3) },
1229+
Token::Str("all"),
1230+
Token::Str("available"),
1231+
Token::Str("unavailable"),
1232+
Token::SeqEnd,
1233+
],
1234+
);
1235+
}
1236+
1237+
#[test]
1238+
fn ser_der_for_transfer_priority() {
1239+
let transfer_priorities = vec![
1240+
TransferPriority::Default,
1241+
TransferPriority::Unimportant,
1242+
TransferPriority::Elevated,
1243+
TransferPriority::Priority,
1244+
];
1245+
assert_tokens(
1246+
&transfer_priorities,
1247+
&[
1248+
Token::Seq { len: Some(4) },
1249+
Token::U8(0),
1250+
Token::U8(1),
1251+
Token::U8(2),
1252+
Token::U8(3),
1253+
Token::SeqEnd,
1254+
],
1255+
);
1256+
}
1257+
1258+
#[test]
1259+
fn der_for_transfer_priority_error() {
1260+
assert_de_tokens_error::<TransferPriority>(
1261+
&[Token::U8(4)],
1262+
"Invalid variant 4, expected 0-3",
1263+
);
1264+
}
1265+
}

0 commit comments

Comments
 (0)