From 997c02b820f670edb8ce776591f11f73b2651172 Mon Sep 17 00:00:00 2001 From: AurelienFT <32803821+AurelienFT@users.noreply.github.com> Date: Tue, 11 Jun 2024 03:01:21 +0200 Subject: [PATCH] Fix cursor block height decoding in SortedTXCursor (#1950) https://github.com/FuelLabs/fuel-core/issues/1825 The `BlockHeight` was encoded in hexadecimal in response of `transactions` endpoint and decoded with `u32::from_str` in parameter of type `SortedTXCursor` which was preventing from making chained API calls. This PR, fixes this by decoding `BlockHeight` from hexadecimal form in `SortedTXCursor`. This bug wasn't catch by the test `tx::get_transactions()` because the test was using only txs included in block 3 that has a string representation that is the same in an hexadecimal and decimal encoding. I added the creation of 10 blocks at start of the test so that now the txs are included in blocks that have a hexadecimal and decimal encoding form that differs. I didn't spotted any hangs on the endpoint when the cursor is initialized manually, however as we can't ask to start at a precise block height without specifying a previous `tx_id` it's not very usable to make manual search. PS: This is my first contribution and so to what I saw my PR doesn't brake anything else and I tried to follow the guidelines of the repository, however maybe their things that I missed and so I'm very open to criticise ! ## Checklist - [x] Breaking changes are clearly marked as such in the PR description and changelog - [x] New behavior is reflected in tests - [x] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [x] I have reviewed the code myself - [x] I have created follow-up issues caused by this PR and linked them here ### After merging, notify other teams [Add or remove entries as needed] - [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/) - [ ] [Sway compiler](https://github.com/FuelLabs/sway/) - [ ] [Platform documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+) (for out-of-organization contributors, the person merging the PR will do this) - [ ] Someone else? --------- Co-authored-by: Green Baneling --- CHANGELOG.md | 3 ++ crates/fuel-core/src/schema/scalars.rs | 5 ++- tests/tests/tx.rs | 45 ++++++++++++++++++++------ 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4360475467..e6e796ec8e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed +- [#1950](https://github.com/FuelLabs/fuel-core/pull/1950): Fix cursor `BlockHeight` encoding in `SortedTXCursor` + ## [Version 0.28.0] ### Changed diff --git a/crates/fuel-core/src/schema/scalars.rs b/crates/fuel-core/src/schema/scalars.rs index 4ead3d0b6e8..f7b51da1009 100644 --- a/crates/fuel-core/src/schema/scalars.rs +++ b/crates/fuel-core/src/schema/scalars.rs @@ -145,9 +145,8 @@ impl CursorType for SortedTxCursor { s.split_once('#').ok_or("Incorrect format provided")?; Ok(Self::new( - u32::from_str(block_height) - .map_err(|_| "Failed to decode block_height")? - .into(), + BlockHeight::from_str(block_height) + .map_err(|_| "Failed to decode block_height")?, Bytes32::decode_cursor(tx_id)?, )) } diff --git a/tests/tests/tx.rs b/tests/tests/tx.rs index 4f16a7ba8fd..26c74091101 100644 --- a/tests/tests/tx.rs +++ b/tests/tests/tx.rs @@ -275,6 +275,19 @@ async fn get_transactions() { let charlie = Address::from([3; 32]); let mut context = TestContext::new(100).await; + // Produce 10 blocks to ensure https://github.com/FuelLabs/fuel-core/issues/1825 is tested + context + .srv + .shared + .poa_adapter + .manually_produce_blocks( + None, + Mode::Blocks { + number_of_blocks: 10, + }, + ) + .await + .expect("Should produce block"); let tx1 = context.transfer(alice, charlie, 1).await.unwrap(); let tx2 = context.transfer(charlie, bob, 2).await.unwrap(); let tx3 = context.transfer(bob, charlie, 3).await.unwrap(); @@ -282,21 +295,35 @@ async fn get_transactions() { let tx5 = context.transfer(charlie, alice, 1).await.unwrap(); let tx6 = context.transfer(alice, charlie, 1).await.unwrap(); - // there are 12 transactions + // Skip the 10 first txs included in the tx pool by the creation of the 10 blocks above + let page_request = PaginationRequest { + cursor: None, + results: 10, + direction: PageDirection::Forward, + }; + let client = context.client; + let response = client.transactions(page_request.clone()).await.unwrap(); + assert_eq!(response.results.len(), 10); + assert!(!response.has_previous_page); + assert!(response.has_next_page); + + // Now, there are 12 transactions // [ // tx1, coinbase_tx1, tx2, coinbase_tx2, tx3, coinbase_tx3, // tx4, coinbase_tx4, tx5, coinbase_tx5, tx6, coinbase_tx6, // ] // Query for first 6: [tx1, coinbase_tx1, tx2, coinbase_tx2, tx3, coinbase_tx3] - let client = context.client; - let page_request = PaginationRequest { - cursor: None, + let first_middle_page_request = PaginationRequest { + cursor: response.cursor.clone(), results: 6, direction: PageDirection::Forward, }; - let response = client.transactions(page_request.clone()).await.unwrap(); + let response = client + .transactions(first_middle_page_request.clone()) + .await + .unwrap(); let transactions = &response .results .iter() @@ -308,9 +335,9 @@ async fn get_transactions() { // coinbase_tx2 assert_eq!(transactions[4], tx3); // coinbase_tx3 - // Check pagination state for first page + // Check pagination state for middle page assert!(response.has_next_page); - assert!(!response.has_previous_page); + assert!(response.has_previous_page); // Query for second page 2 with last given cursor: [tx4, coinbase_tx4, tx5, coinbase_tx5] let page_request_middle_page = PaginationRequest { @@ -319,10 +346,10 @@ async fn get_transactions() { direction: PageDirection::Forward, }; - // Query backwards from last given cursor [3]: [tx3, coinbase_tx2, tx2, coinbase_tx1, tx1] + // Query backwards from last given cursor [3]: [tx3, coinbase_tx2, tx2, coinbase_tx1, tx1, tx_block_creation_10, tx_block_creation_9, ...] let page_request_backwards = PaginationRequest { cursor: response.cursor.clone(), - results: 6, + results: 16, direction: PageDirection::Backward, };