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

load CommonAccountAddress with sync start #883

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

anarcher
Copy link
Contributor

Github Issue

common/Config.CommonAccountAddress is not loaded when common.Config is created. So sync has to load CommonAccountAddress and add it to Config.CommonAccountAddress

Background

Block height 348204 has inflation-pf operation, and this operation validation needs common.Config.CommonAccountAddress.

Solution

sync/Config loads CommonAccountAddress and add it to common/Config.CommonAccountAddress.

Possible Drawbacks

IHMO, common/Config itself loads it's own attributes when it is created makes sense.

@anarcher anarcher added bug Something isn't working sync labels Dec 20, 2018
@anarcher anarcher self-assigned this Dec 20, 2018
@codecov-io
Copy link

codecov-io commented Dec 20, 2018

Codecov Report

Merging #883 into master will increase coverage by 0.1%.
The diff coverage is 57.14%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #883     +/-   ##
========================================
+ Coverage    60.4%   60.5%   +0.1%     
========================================
  Files         153     153             
  Lines       10713   10724     +11     
========================================
+ Hits         6471    6489     +18     
+ Misses       3509    3501      -8     
- Partials      733     734      +1
Flag Coverage Δ
#integration_tests_long_term 44.56% <57.14%> (+0.1%) ⬆️
#integration_tests_node 40.44% <57.14%> (+0.06%) ⬆️
#unittests 48.63% <50%> (-0.01%) ⬇️
Impacted Files Coverage Δ
cmd/sebak/cmd/run.go 57.52% <33.33%> (-0.21%) ⬇️
lib/sync/config.go 76.74% <63.63%> (-2.48%) ⬇️
lib/network/validator_connection_manager.go 88.53% <0%> (+4.34%) ⬆️
lib/common/message.go 42.1% <0%> (+10.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b594652...a9f3384. Read the comment docs.

1 similar comment
@codecov-io
Copy link

Codecov Report

Merging #883 into master will increase coverage by 0.1%.
The diff coverage is 57.14%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #883     +/-   ##
========================================
+ Coverage    60.4%   60.5%   +0.1%     
========================================
  Files         153     153             
  Lines       10713   10724     +11     
========================================
+ Hits         6471    6489     +18     
+ Misses       3509    3501      -8     
- Partials      733     734      +1
Flag Coverage Δ
#integration_tests_long_term 44.56% <57.14%> (+0.1%) ⬆️
#integration_tests_node 40.44% <57.14%> (+0.06%) ⬆️
#unittests 48.63% <50%> (-0.01%) ⬇️
Impacted Files Coverage Δ
cmd/sebak/cmd/run.go 57.52% <33.33%> (-0.21%) ⬇️
lib/sync/config.go 76.74% <63.63%> (-2.48%) ⬇️
lib/network/validator_connection_manager.go 88.53% <0%> (+4.34%) ⬆️
lib/common/message.go 42.1% <0%> (+10.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b594652...a9f3384. Read the comment docs.

@anarcher anarcher modified the milestones: TBD, MainNet Dec 20, 2018
Copy link
Member

@Charleslee522 Charleslee522 left a comment

Choose a reason for hiding this comment

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

LGTM

@soonkuk
Copy link
Member

soonkuk commented Dec 21, 2018

LGTM

@anarcher anarcher merged commit d5bdeff into bosnet:master Dec 21, 2018
@anarcher anarcher deleted the fix-common-acc-addr branch December 21, 2018 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sync Urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants