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

[TCB] building the TCB with overflow checks #6533

Closed
wants to merge 1 commit into from
Closed

Conversation

mimoo
Copy link
Contributor

@mimoo mimoo commented Oct 15, 2020

This proposes to build the TCB with overflow checks.

How can I test this PR?

@bors-libra bors-libra added this to In Review in bors Oct 15, 2020
@mimoo
Copy link
Contributor Author

mimoo commented Nov 12, 2020

@davidiw looks like nothing is happening in longevity testing so far in terms of integer overflow, perhaps a good time to revisit this PR?

@davidiw
Copy link
Contributor

davidiw commented Nov 30, 2020

Fix the linters, I think you need to either eliminate the last line in the patch. Let's verify all is good with a cluster test canary too. PR looks pretty interesting but good enough. Though might be good to get a dev-infra person to weigh in.

@mimoo
Copy link
Contributor Author

mimoo commented Dec 1, 2020

It's ugly but without custom profiles (cf rust-lang/cargo#6988) I don't see a better way to do it : o would be interested to know what the rust experts think : ) (cc @bmwill @sunshowers @metajack @phlip9 )

@mimoo
Copy link
Contributor Author

mimoo commented Dec 1, 2020

/canary

bors-libra pushed a commit that referenced this pull request Dec 1, 2020
@bors-libra bors-libra moved this from In Review to Canary in bors Dec 1, 2020
@bors-libra
Copy link
Contributor

💔 Test Failed - commit-workflow

@bors-libra bors-libra moved this from Canary to In Review in bors Dec 1, 2020
@github-actions
Copy link

github-actions bot commented Dec 1, 2020

Cluster Test Result

Compatibility test results for land_52f36fdf ==> land_f9565cc8 (PR)
1. All instances running land_52f36fdf, generating some traffic on network
2. First validator land_52f36fdf ==> land_f9565cc8, to validate storage
3. First batch validators (14) land_52f36fdf ==> land_f9565cc8, to test consensus
4. Second batch validators (15) land_52f36fdf ==> land_f9565cc8, to upgrade rest of the validators
5. All full nodes (30) land_52f36fdf ==> land_f9565cc8, to finish the network upgrade
all up : 1022 TPS, 4426 ms latency, 5400 ms p99 latency, no expired txns
Logs: http://kibana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-12-01T06:04:27Z',to:'2020-12-01T06:33:29Z'))
Dashboard: http://grafana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/d/2XqUIhnWz/performance?from=1606802667000&to=1606804409000
Validator 1 logs: http://kibana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2020-12-01T06:04:27Z',to:'2020-12-01T06:33:29Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_52f36fdf --cluster-test-tag land_f9565cc8 -E RUST_LOG=debug -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_f9565cc8 --report report.json --suite land_blocking_compat

🎉 Land-blocking cluster test passed! 👌

Copy link
Contributor

@davidiw davidiw left a comment

Choose a reason for hiding this comment

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

god save us all

@davidiw
Copy link
Contributor

davidiw commented Dec 22, 2020

let's not leave approved, stale PRs unclosed.

@davidiw davidiw closed this Dec 22, 2020
@bors-libra bors-libra removed this from In Review in bors Dec 22, 2020
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.

None yet

3 participants