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

go/registry: Handle the old and busted node descriptor envelope #2614

Merged
merged 1 commit into from
Jan 29, 2020

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Jan 29, 2020

Crawling in my skin
These wounds, they will not heal

@Yawning Yawning added c:registry Category: entity/node/runtime registry service c:security Category: security sensitive labels Jan 29, 2020
@Yawning Yawning self-assigned this Jan 29, 2020
@Yawning Yawning force-pushed the yawning/hack/node-backward-compat branch from 314ad5b to 1ae026d Compare January 29, 2020 12:48
@Yawning Yawning force-pushed the yawning/hack/node-backward-compat branch 2 times, most recently from 5b88c3d to d3c9a4e Compare January 29, 2020 13:44
@codecov
Copy link

codecov bot commented Jan 29, 2020

Codecov Report

Merging #2614 into master will decrease coverage by 0.15%.
The diff coverage is 20.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2614      +/-   ##
==========================================
- Coverage   63.36%   63.21%   -0.16%     
==========================================
  Files         359      360       +1     
  Lines       33864    33963      +99     
==========================================
+ Hits        21459    21469      +10     
- Misses       9766     9849      +83     
- Partials     2639     2645       +6
Impacted Files Coverage Δ
go/consensus/tendermint/apps/registry/genesis.go 52.13% <0%> (-0.91%) ⬇️
go/oasis-node/cmd/debug/debug.go 100% <100%> (ø) ⬆️
...nsus/tendermint/apps/supplementarysanity/checks.go 51.92% <100%> (+1.46%) ⬆️
go/registry/api/api.go 37.9% <31.91%> (-0.02%) ⬇️
go/registry/api/sanity_check.go 59.2% <44.44%> (-2.67%) ⬇️
go/oasis-node/cmd/debug/fixgenesis/fixgenesis.go 8.97% <8.97%> (ø)
go/storage/metrics.go 73.21% <0%> (-5.36%) ⬇️
go/storage/api/root_cache.go 70.11% <0%> (-4.6%) ⬇️
go/storage/database/database.go 77.22% <0%> (-3.97%) ⬇️
go/storage/mkvs/urkel/urkel.go 84.61% <0%> (-3.85%) ⬇️
... and 20 more

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 56e6d7f...b968e5d. Read the comment docs.

@Yawning Yawning force-pushed the yawning/hack/node-backward-compat branch from d3c9a4e to 08775c5 Compare January 29, 2020 13:57
@Yawning Yawning marked this pull request as ready for review January 29, 2020 13:58
> Crawling in my skin
> These wounds, they will not heal
@Yawning Yawning force-pushed the yawning/hack/node-backward-compat branch from 08775c5 to b968e5d Compare January 29, 2020 14:28
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Looks good as long as we revert it soon after.

var nsn node.MultiSignedNode
nsn.MultiSigned.Signatures = []signature.Signature{osn.Signed.Signature}

// TODO: Someone that understands the issue can also fix the node
Copy link
Member

Choose a reason for hiding this comment

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

We just need to clear all flags and set the (new) validator flag (other nodes are not persisted anyway AFAIK).

@Yawning Yawning merged commit 1d4519b into master Jan 29, 2020
@Yawning Yawning deleted the yawning/hack/node-backward-compat branch January 29, 2020 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:registry Category: entity/node/runtime registry service c:security Category: security sensitive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants