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

Update to go 1.14 #4947

Merged
merged 93 commits into from
Mar 14, 2020
Merged

Update to go 1.14 #4947

merged 93 commits into from
Mar 14, 2020

Conversation

prestonvanloon
Copy link
Member

@prestonvanloon prestonvanloon commented Feb 26, 2020

https://blog.golang.org/go1.14

Also replaced the cache here with LRU since tests were too flaky or failing with a sleep delay.

@prestonvanloon prestonvanloon added Ready For Review A pull request ready for code review Blocked Blocked by research or external factors and removed Ready For Review A pull request ready for code review labels Feb 26, 2020
@prestonvanloon
Copy link
Member Author

Blocked by bazel-contrib/rules_go#2387

@prestonvanloon
Copy link
Member Author

No longer blocked, fixed in bazel-contrib/rules_go#2388.

@prestonvanloon prestonvanloon added Ready For Review A pull request ready for code review and removed Blocked Blocked by research or external factors In Progress labels Feb 26, 2020
@prestonvanloon prestonvanloon added Blocked Blocked by research or external factors In Progress and removed Ready For Review A pull request ready for code review labels Feb 26, 2020
@prestonvanloon
Copy link
Member Author

Blocked again by an unsafe pointer conversion in upstream dependency. Trying to update it now...


fatal error: checkptr: unsafe pointer conversion
--
  |  
  | goroutine 1 [running, locked to thread]:
  | runtime.throw(0x1b2a0c1, 0x23)
  | GOROOT/src/runtime/panic.go:1112 +0x72 fp=0xc0007f7ac8 sp=0xc0007f7a98 pc=0x4762f2
  | runtime.checkptrAlignment(0xc00015d429, 0x190c260, 0x1)
  | GOROOT/src/runtime/checkptr.go:13 +0xd0 fp=0xc0007f7af8 sp=0xc0007f7ac8 pc=0x447540
  | golang.org/x/crypto/sha3.xorInUnaligned(0xc00015d340, 0xc00015d429, 0x88, 0xa8)
  | external/org_golang_x_crypto/sha3/xor_unaligned.go:13 +0x60 fp=0xc0007f7b30 sp=0xc0007f7af8 pc=0xe0b300
  | golang.org/x/crypto/sha3.(*state).permute(0xc00015d340)
  | external/org_golang_x_crypto/sha3/sha3.go:84 +0x1d3 fp=0xc0007f7b80 sp=0xc0007f7b30 pc=0xe09943
  | golang.org/x/crypto/sha3.(*state).padAndPermute(0xc00015d340, 0x29f3401)
  | external/org_golang_x_crypto/sha3/sha3.go:117 +0x347 fp=0xc0007f7c28 sp=0xc0007f7b80 pc=0xe09d07
  | golang.org/x/crypto/sha3.(*state).Read(0xc00015d340, 0xc000630640, 0x20, 0x20, 0x7f7ce631e560, 0x0, 0xc00015d180)
  | external/org_golang_x_crypto/sha3/sha3.go:163 +0x2e0 fp=0xc0007f7cb8 sp=0xc0007f7c28 pc=0xe0a7d0
  | golang.org/x/crypto/sha3.(*state).Sum(0xc00015d180, 0xc000630620, 0x0, 0x20, 0x0, 0x0, 0x0)
  | external/org_golang_x_crypto/sha3/sha3.go:190 +0x233 fp=0xc0007f7d50 sp=0xc0007f7cb8 pc=0xe0aa33
  | github.com/ethereum/go-ethereum/crypto.Keccak256Hash(0xc00063cca0, 0x1, 0x1, 0x6ef8c092e64583ff, 0xc0ad6c991be0485b, 0x21b463e3b52f6201, 0x0)
  | external/com_github_ethereum_go_ethereum/crypto/crypto.go:69 +0x1b6 fp=0xc0007f7db8 sp=0xc0007f7d50 pc=0x13c8786
  | github.com/ethereum/go-ethereum/trie.init()
  | external/com_github_ethereum_go_ethereum/trie/trie.go:34 +0xa8a fp=0xc0007f7e98 sp=0xc0007f7db8 pc=0x153048a


@prestonvanloon
Copy link
Member Author

Seems like boltdb has this issue too. I'll put this on hold for now.

@prestonvanloon
Copy link
Member Author

One solution is to upgrade to bbolt, however the issue exists there too. Blocked by etcd-io/bbolt#187 for now.

@prestonvanloon
Copy link
Member Author

Unblocked!

@prestonvanloon prestonvanloon added Ready For Review A pull request ready for code review and removed Blocked Blocked by research or external factors labels Mar 13, 2020
@codecov
Copy link

codecov bot commented Mar 13, 2020

Codecov Report

Merging #4947 into master will decrease coverage by 26.57%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #4947       +/-   ##
==========================================
- Coverage   35.46%   8.89%   -26.58%     
==========================================
  Files         219      47      -172     
  Lines       17297    3709    -13588     
==========================================
- Hits         6135     330     -5805     
+ Misses      10138    3323     -6815     
+ Partials     1024      56      -968

MaxCost: forkChoiceProcessedRootsSize,
BufferItems: 64,
})
cache, err := lru.New(forkChoiceProcessedRootsSize)
Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok to bundle this change in the same PR, perhaps just mention it in the description?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh yes good idea

@prylabs-bulldozer prylabs-bulldozer bot merged commit a0b142a into master Mar 14, 2020
@delete-merged-branch delete-merged-branch bot deleted the go-1.14 branch March 14, 2020 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants