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.

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

Fix transaction broadcast #1407

Merged
merged 20 commits into from
Dec 14, 2022
Merged

Fix transaction broadcast #1407

merged 20 commits into from
Dec 14, 2022

Conversation

Harrm
Copy link
Contributor

@Harrm Harrm commented Nov 9, 2022

Referenced issues

resolves #1324

Description of the Change

Due to a typo in broadcasting code, messages were not delivered when there were no open outgoing streams to the target peer, which was always the case when broadcasting transactions.

Benefits

Transaction broadcasting works.

Possible Drawbacks

None expected.

Usage Examples or Tests

Alternate Designs

@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #1407 (01607c7) into master (2b6d360) will increase coverage by 0.10%.
The diff coverage is 19.30%.

❗ Current head 01607c7 differs from pull request most recent head e234192. Consider uploading reports for the commit e234192 to get more accurate results

@@            Coverage Diff             @@
##           master    #1407      +/-   ##
==========================================
+ Coverage   24.31%   24.41%   +0.10%     
==========================================
  Files         643      639       -4     
  Lines       24260    24132     -128     
  Branches    12577    12518      -59     
==========================================
- Hits         5899     5893       -6     
+ Misses      13096    12983     -113     
+ Partials     5265     5256       -9     
Impacted Files Coverage Δ
core/api/service/author/impl/author_api_impl.cpp 33.01% <0.00%> (ø)
core/api/service/impl/api_service_impl.cpp 13.33% <0.00%> (-0.14%) ⬇️
core/application/app_configuration.hpp 100.00% <ø> (ø)
core/application/impl/app_configuration_impl.hpp 26.92% <0.00%> (ø)
...onsensus/authority/impl/authority_manager_impl.cpp 21.29% <0.00%> (ø)
core/consensus/babe/impl/block_executor_impl.cpp 23.39% <0.00%> (-0.28%) ⬇️
...nsensus/grandpa/impl/vote_crypto_provider_impl.cpp 0.00% <0.00%> (ø)
core/network/helpers/stream_read_buffer.hpp 0.00% <0.00%> (ø)
core/network/impl/peer_manager_impl.cpp 3.05% <0.00%> (-0.21%) ⬇️
...network/impl/protocols/block_announce_protocol.cpp 0.00% <ø> (ø)
... and 30 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Harrm Harrm requested review from xDimon and turuslan November 15, 2022 12:14
@Harrm Harrm marked this pull request as ready for review November 15, 2022 12:14
@Harrm Harrm merged commit 526b774 into master Dec 14, 2022
@Harrm Harrm deleted the fix/transaction-broadcast branch December 14, 2022 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix extrinsics status in polkadot js
3 participants