Skip to content

Commit

Permalink
Fix cursor block height decoding in SortedTXCursor (#1950)
Browse files Browse the repository at this point in the history
#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 <XgreenX9999@gmail.com>
  • Loading branch information
AurelienFT and xgreenx committed Jun 11, 2024
1 parent 7871362 commit 997c02b
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions crates/fuel-core/src/schema/scalars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?,
))
}
Expand Down
45 changes: 36 additions & 9 deletions tests/tests/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,28 +275,55 @@ 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();
let tx4 = context.transfer(bob, charlie, 3).await.unwrap();
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()
Expand All @@ -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 {
Expand All @@ -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,
};

Expand Down

0 comments on commit 997c02b

Please sign in to comment.