Make transaction status stream work #1108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
We'll likely also want to consider this issue as well, because without a terminal message indicating the closing of the stream it makes error handling on the client side more difficult: #823 |
Voxelot
reviewed
Apr 6, 2023
Voxelot
reviewed
Apr 20, 2023
# Conflicts: # Cargo.lock # tests/Cargo.toml
…freesig/test-submit-await
xgreenx
reviewed
May 26, 2023
| let (status_sender, _) = broadcast::channel(capacity); | ||
| let (update_sender, _) = broadcast::channel(capacity); | ||
| let (new_tx_notification_sender, _) = broadcast::channel(capacity); | ||
| let update_sender = UpdateSender::new(capacity); |
There was a problem hiding this comment.
We use 100 as capacity right now. It means we can only submit 100 transactions because of the permit. I think we need to increase it or make configurable
freesig
commented
May 29, 2023
| @@ -147,7 +148,10 @@ impl fuel_core_txpool::ports::TxPoolDb for Database { | |||
| fn transaction_status( | |||
| &self, | |||
| tx_id: &fuel_core_types::fuel_types::Bytes32, | |||
| ) -> StorageResult<Option<fuel_core_types::services::txpool::TransactionStatus>> { | |||
| Ok(self.get_tx_status(tx_id)?) | |||
| ) -> StorageResult<fuel_core_types::services::txpool::TransactionStatus> { | |||
There was a problem hiding this comment.
Kinda weird to return an error for not found
Merged
xgreenx
added a commit
that referenced
this pull request
Jun 14, 2023
## What's Changed * version compatibility cleanup by @Voxelot in #1171 * Added example with custom query around the `fuel-core-client` by @xgreenx in #1175 * Update to fuel-vm 0.32 (including wideint gas profiling) by @Dentosal in #1173 * feat: Client primitives by @bvrooman in #1144 * Improve executor config by @Voxelot in #1185 * Added `contract_id` to the `ContractConfig` by @xgreenx in #1184 * fix windows file name error by @firedpeanut in #1176 * Make transaction status stream work by @freesig in #1108 * Added `submit_and_await` endpoint to not miss the notifications by @xgreenx in #1192 * feat: Use ID types in client api by @bvrooman in #1191 * Use `fuel-vm 0.33` with predicate estimation by @xgreenx in #1195 * Add transaction lifecycle diagram to the docs by @digorithm in #1201 * sync with peers before producing blocks by @leviathanbeak in #1169 * SMT storage key hashing by @xgreenx in #1207 ## New Contributors * @firedpeanut made their first contribution in #1176 **Full Changelog**: v0.18.1...v0.19.0 --------- Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
crypto523
pushed a commit
to crypto523/fuel-core
that referenced
this pull request
Oct 7, 2024
## What's Changed * version compatibility cleanup by @Voxelot in FuelLabs/fuel-core#1171 * Added example with custom query around the `fuel-core-client` by @xgreenx in FuelLabs/fuel-core#1175 * Update to fuel-vm 0.32 (including wideint gas profiling) by @Dentosal in FuelLabs/fuel-core#1173 * feat: Client primitives by @bvrooman in FuelLabs/fuel-core#1144 * Improve executor config by @Voxelot in FuelLabs/fuel-core#1185 * Added `contract_id` to the `ContractConfig` by @xgreenx in FuelLabs/fuel-core#1184 * fix windows file name error by @firedpeanut in FuelLabs/fuel-core#1176 * Make transaction status stream work by @freesig in FuelLabs/fuel-core#1108 * Added `submit_and_await` endpoint to not miss the notifications by @xgreenx in FuelLabs/fuel-core#1192 * feat: Use ID types in client api by @bvrooman in FuelLabs/fuel-core#1191 * Use `fuel-vm 0.33` with predicate estimation by @xgreenx in FuelLabs/fuel-core#1195 * Add transaction lifecycle diagram to the docs by @digorithm in FuelLabs/fuel-core#1201 * sync with peers before producing blocks by @leviathanbeak in FuelLabs/fuel-core#1169 * SMT storage key hashing by @xgreenx in FuelLabs/fuel-core#1207 ## New Contributors * @firedpeanut made their first contribution in FuelLabs/fuel-core#1176 **Full Changelog**: FuelLabs/fuel-core@v0.18.1...v0.19.0 --------- Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the
transaction_status_changefunction to improve its readability and efficiency. The main changes are:BoxStream<'a, Result<TxUpdate, BroadcastStreamRecvError>>toBoxStream<'a, TxStatusMessage>.impl Stream<Item = anyhow::Result<TransactionStatus>> + 'atoimpl Stream<Item = anyhow::Result<ApiTxStatus>> + 'a.Additionally, this PR introduces property testing to ensure the refactored
transaction_status_changefunction behaves correctly under various input conditions. This will help identify potential edge cases, increase the test coverage, and provide greater confidence in the robustness of the changes made.