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

feat: add foreign proposal validation #889

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

Cifko
Copy link
Contributor

@Cifko Cifko commented Jan 12, 2024

Description

Do most of the validation for foreign proposals as their are done on the local proposals.
I had to move the tokio::select from on_inbound_message to the parent tokio select, so it will stop being cancelled. I tried to fuse it instead but then I got into mutability concurrency issue.

Motivation and Context

Fixes #887

How Has This Been Tested?

Running dan-testing and looking at the logs. The requester abandoned request is gone.

What process can a PR reviewer use to test or verify this change?

Same as above.

Breaking Changes

  • None
  • Requires data directory to be deleted
  • Other - Please specify

Copy link

Test Results (CI)

194 tests  ±0   194 ✅ +5   1h 26m 30s ⏱️ - 1h 19m 19s
 52 suites ±0     0 💤 ±0 
  2 files   ±0     0 ❌  - 5 

Results for commit d0a0827. ± Comparison against base commit b7ccff2.

Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing that issue - might have to refactor later but getting this issue fixed is higher priority

@sdbondi sdbondi added this pull request to the merge queue Jan 12, 2024
Merged via the queue into tari-project:development with commit 76bf409 Jan 12, 2024
11 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jan 16, 2024
Description
---
Move HS message receivers into worker, pass messages to the
OnInboundMessage handler. This checks proposals for missing transactions
and parks them if necessary. New transaction notifications are sent to
the OnInboundMessage handler to check if any proposals can be unparked.
Once unparked, they are added to the "next message buffer". The
message buffer is polled (cancel safe) in the consensus worker tokio
select loop.

Motivation and Context
---
Clean up after fix in #889  

How Has This Been Tested?
---
Running stress test from PR #880 

What process can a PR reviewer use to test or verify this change?
---
Consensus should still work as before

Breaking Changes
---

- [x] None
- [ ] Requires data directory to be deleted
- [ ] Other - Please specify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(consensus): inbound message handler future can be aborted resulting in dropped messages and blocks
3 participants