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.

Sign up for GitHub

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

Jump to bottom

Welcome node should forward tx if not synced#872 #896

Merged
merged 3 commits into from
Mar 8, 2023
Merged

Welcome node should forward tx if not synced#872 #896

merged 3 commits into from
Mar 8, 2023

Conversation

apoorv-2204
Copy link
Contributor

@apoorv-2204 apoorv-2204 commented Feb 7, 2023 •
edited
Loading

Description

When it is the time for protocol to create a new node renewal transaction, there exist a time window where new transactions arrive and election results with anomaly due to missing latest Node shared secret transaction.

Solutions and Further improvements

  • Check for whether a node is synced for every transaction
  • If not schedule a request for resync.
  • Last nss transaction validation stamp and last scheduling date to determine node status.

Drawbacks

  • NSS chain has to be checked for each transaction

Solution

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • manual

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@apoorv-2204 apoorv-2204 self-assigned this Feb 7, 2023
@apoorv-2204 apoorv-2204 added the bug Something isn't working label Feb 7, 2023
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch 2 times, most recently from 4907215 to bc52134 Compare February 8, 2023 10:37
@apoorv-2204 apoorv-2204 marked this pull request as ready for review February 8, 2023 10:45
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch 6 times, most recently from d09e84e to 0db071a Compare February 15, 2023 12:57
@samuelmanzanera samuelmanzanera linked an issue Feb 21, 2023 that may be closed by this pull request
Welcome node should forward tx if not synced #872
Closed
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 0db071a to 32a1bb9 Compare February 23, 2023 12:06
Neylix
Neylix requested changes Feb 24, 2023
View reviewed changes
lib/archethic.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
lib/archethic/self_repair/resync.ex Outdated Show resolved Hide resolved
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 32a1bb9 to 5a0d14e Compare February 25, 2023 17:20
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 5a0d14e to d4803b1 Compare February 25, 2023 17:24
Neylix
Neylix requested changes Feb 27, 2023
View reviewed changes
lib/archethic.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
lib/archethic.ex Outdated Show resolved Hide resolved
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from d4803b1 to ec6e579 Compare February 27, 2023 17:03
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from ec6e579 to a67b853 Compare February 28, 2023 09:45
Neylix
Neylix requested changes Feb 28, 2023
View reviewed changes
lib/archethic.ex Outdated Show resolved Hide resolved
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 297d9f2 to 0b7edc5 Compare February 28, 2023 15:27
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 0b7edc5 to 122914d Compare March 2, 2023 14:10
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 122914d to cf68261 Compare March 2, 2023 14:11
@apoorv-2204 apoorv-2204 requested a review from Neylix March 2, 2023 14:11
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from cf68261 to 6e67286 Compare March 2, 2023 15:56
samuelmanzanera
samuelmanzanera requested changes Mar 6, 2023
View reviewed changes
lib/archethic/crypto.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
config/config.exs Show resolved Hide resolved
lib/archethic_web/channels/user_socket.ex Show resolved Hide resolved
lib/archethic/self_repair/resync.ex Outdated Show resolved Hide resolved
lib/archethic/self_repair/resync.ex Outdated Show resolved Hide resolved
lib/archethic/shared_secrets.ex Show resolved Hide resolved
lib/archethic/shared_secrets.ex Show resolved Hide resolved
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 6e67286 to 4f0daa8 Compare March 6, 2023 17:58
bchamagne
bchamagne reviewed Mar 7, 2023
View reviewed changes
test/archethic_test.exs Outdated Show resolved Hide resolved
Neylix
Neylix requested changes Mar 7, 2023
View reviewed changes
lib/archethic/self_repair.ex Outdated Show resolved Hide resolved
Neylix
Neylix requested changes Mar 7, 2023
View reviewed changes
lib/archethic/shared_secrets.ex Outdated Show resolved Hide resolved
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 4f0daa8 to 01eb6cc Compare March 7, 2023 13:56
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch 2 times, most recently from d6b9b45 to 186ca84 Compare March 7, 2023 17:05
@apoorv-2204
Add new mix aliases command ,Resolve failure in git hooks.
2178b45
@apoorv-2204
Handling new tx when node shared_secrets is not synced
55d77e0 
 - verify whether node is synced ?
    - last scheduling time is not too old
    - manually ask network

 - Augment Message.NewTransaction with a welcome node key

 - update tests to handle new tx with protocol chain NSS synchronization
@apoorv-2204
Resolves Dialyzer Warning and test failures
73dd513 
- improve aliasing of archethic modules
- Dialyzer warning was due to a missing archethic alias
- Oracle/Reward Scheduler test failures were due to a persistent term not being nil
or cleared after tests
- Graphql schema test crashes : add message reply for get Beacon aggreagte
@apoorv-2204 apoorv-2204 force-pushed the welcome_node_should_forward_tx_if_not_synced#872 branch from 186ca84 to 73dd513 Compare March 7, 2023 18:24
samuelmanzanera
samuelmanzanera approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@samuelmanzanera samuelmanzanera left a comment

Choose a reason for hiding this comment

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

Code is better now, thanks for the refactor !

@Neylix Neylix merged commit 4408b34 into archethic-foundation:develop Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelmanzanera samuelmanzanera samuelmanzanera approved these changes

@Neylix Neylix Neylix approved these changes

@bchamagne bchamagne Awaiting requested review from bchamagne

Assignees

@apoorv-2204 apoorv-2204

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Welcome node should forward tx if not synced
4 participants
@apoorv-2204 @samuelmanzanera @Neylix @bchamagne