[1.0.2] Don't assume that claimed block is in fork_db. #788

greg7mdp · 2024-09-16T20:56:11Z

Resolves #778.

Update fork_db_validated_block_exists to lookup parents instead of just claimed block.

Also updated the validated member in both block_state_legacy and block_state to be copyable_atomic<bool> instead of bool, which allowed to make the set_valid() and is_valid() members public, and thus eliminate a couple of friend declarations.

…just claimed block.

libraries/chain/fork_database.cpp

libraries/chain/controller.cpp

libraries/chain/fork_database.cpp

libraries/chain/include/eosio/chain/block_state_legacy.hpp

…assert not always true)

…ations.

ericpassmore · 2024-09-17T21:01:58Z

Note:start
category: Other
component: Internal
summary: Take into account that a claimed block may already be irreversible in consider_voting.
Note:end

unittests/fork_db_tests.cpp

libraries/chain/fork_database.cpp

…tors are as well.

…in another branch)

libraries/chain/include/eosio/chain/block_state_legacy.hpp

arhag

@greg7mdp:

See #751 (comment) for the test case that works for both #751 and #778.

I think that test should be added here to demonstrate that this PR does in fact resolve the bug reported in #778 properly.

greg7mdp · 2024-10-01T18:45:43Z

I think that test should be added here to demonstrate that this PR does in fact resolve the bug reported in #778 properly.

done.

greg7mdp · 2024-10-02T00:07:30Z

@arhag got two new approvals from Lin and Kevin. Time to merge?

Free to merge

greg7mdp added 3 commits September 16, 2024 16:52

Update fork_db_validated_block_exists to lookup parents instead of …

771736e

…just claimed block.

Add comment.

6efb76d

Undo previous change

01bf3ef

greg7mdp requested review from heifner and linh2931 September 16, 2024 21:01

heifner requested changes Sep 16, 2024

View reviewed changes

libraries/chain/fork_database.cpp Outdated Show resolved Hide resolved

libraries/chain/controller.cpp Outdated Show resolved Hide resolved

libraries/chain/controller.cpp Outdated Show resolved Hide resolved

libraries/chain/controller.cpp Show resolved Hide resolved

greg7mdp added 3 commits September 16, 2024 18:39

Fix incorrect root check in validated_block_exists_impl

c4cd685

Make is_valid() thread-safe for block_state_legacy.

3c417b1

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

e2a7a72

greg7mdp changed the base branch from main to release/1.0 September 16, 2024 22:58

heifner reviewed Sep 16, 2024

View reviewed changes

libraries/chain/fork_database.cpp Outdated Show resolved Hide resolved

heifner reviewed Sep 16, 2024

View reviewed changes

libraries/chain/include/eosio/chain/block_state_legacy.hpp Show resolved Hide resolved

greg7mdp added 7 commits September 16, 2024 19:30

Update validated_block_exists_impl to take two ids as suggested.

bc9a5d8

Update comment

54f626f

Fix issue in validated_block_exists_impl when id is not present (…

3a133d2

…assert not always true)

Make is_valid and set_valid public and remove two friend declar…

311c94d

…ations.

Make set_valid() and is_valid() public for block_state as well.

a471e95

Add unit test for validated_block_exists

8f36108

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

3519806

ericpassmore added the bug The product is not working as was intended. label Sep 17, 2024

heifner reviewed Sep 17, 2024

View reviewed changes

unittests/fork_db_tests.cpp Show resolved Hide resolved

Add testcase with incorrect call.

9bde458

heifner requested changes Sep 18, 2024

View reviewed changes

greg7mdp added 5 commits September 18, 2024 08:40

Update test to reflect that if a block is valid in fork_db, its ances…

45e67aa

…tors are as well.

Validate precondition in case we return true (claimed_id cannot be …

2cacd26

…in another branch)

Add comment

15ecf69

Remove unnecessary check.

9074bab

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

1ca07ae

heifner approved these changes Sep 18, 2024

View reviewed changes

libraries/chain/include/eosio/chain/block_state_legacy.hpp Show resolved Hide resolved

heifner changed the title ~~Don't assume that claimed block is in fork_db.~~ [1.0.2] Don't assume that claimed block is in fork_db. Sep 24, 2024

arhag previously requested changes Sep 25, 2024

View reviewed changes

greg7mdp added 13 commits September 25, 2024 17:41

Support setting partitions temporarily using fc::scoped_set_value

241b8df

Update set_partition to take list of nodes instead of indices.

11ceb73

Store votes from each node so we can simulate slow propagation.

c64ebf7

First part of the new test works.

198a35f

wip

6b39978

Finish test finality_advancing_past_block_claimed_on_alternate_branch

1ed13f4

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_751_2

4d45f87

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

ef2bd88

If previous block is the root it is validated as well.

b1045ba

Update test to match scenarion from issue #751.

02f0f3a

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_751_2

f2a6d07

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

7336429

Add diagram showing timeline for A, B C and D receiving blocks,

3731f83

greg7mdp mentioned this pull request Oct 1, 2024

[1.0.2] Add new testcase where finality advances on alternate branch with lesser lib #851

Closed

greg7mdp added 2 commits October 1, 2024 14:41

Alignment in comment

2abb404

Merge remote-tracking branch 'origin/gh_751_2' into gh_778

8985ec5

greg7mdp added 2 commits October 1, 2024 15:04

Comment out _debug_mode

60eab18

Merge remote-tracking branch 'origin/gh_751_2' into gh_778

a23a234

arhag linked an issue Oct 1, 2024 that may be closed by this pull request

Add new testcase where finality advances on alternate branch with lesser lib #751

Closed

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

2f9116d

heifner approved these changes Oct 1, 2024

View reviewed changes

linh2931 approved these changes Oct 1, 2024

View reviewed changes

Merge branch 'release/1.0' of github.com:AntelopeIO/spring into gh_778

0b8f1cf

greg7mdp merged commit 35a9bae into release/1.0 Oct 2, 2024
36 checks passed

greg7mdp deleted the gh_778 branch October 2, 2024 12:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1.0.2] Don't assume that claimed block is in fork_db. #788

[1.0.2] Don't assume that claimed block is in fork_db. #788

greg7mdp commented Sep 16, 2024 •

edited

Loading

ericpassmore commented Sep 17, 2024 •

edited

Loading

arhag left a comment

greg7mdp commented Oct 1, 2024

greg7mdp commented Oct 2, 2024

[1.0.2] Don't assume that claimed block is in fork_db. #788

[1.0.2] Don't assume that claimed block is in fork_db. #788

Conversation

greg7mdp commented Sep 16, 2024 • edited Loading

ericpassmore commented Sep 17, 2024 • edited Loading

arhag left a comment

Choose a reason for hiding this comment

greg7mdp commented Oct 1, 2024

greg7mdp commented Oct 2, 2024

greg7mdp commented Sep 16, 2024 •

edited

Loading

ericpassmore commented Sep 17, 2024 •

edited

Loading