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

Support for VRF implementation with libsodium #297

Merged
merged 4 commits into from
Sep 3, 2021

Conversation

torao
Copy link
Contributor

@torao torao commented Aug 4, 2021

Description

This PR supports the use of libsodium in addition to r2ishiguro, which has been used in Ostracon so far. Note that the build-time default is still r2ishiguro. Setting LIBSODIUM=1 at build or test time will enable libsodium. The fix to make the build default to libsodium is in another PR.

Note

In v0.34.8, I found a bug that when restoring Evidence from evidencePool, it failed to unmarshal as tmproto.LightClientAttackEvidence because it was marshaled by tmproto.Evidence. This bug has been fixed in v0.34.11. Unfortunately, there were two checks (1) and (2) that happened to work correctly due to the side effects of this bug. Since these checks don't appear to have the correct semantics and have been removed in v0.34.11, I remove them in this PR. Related PR in Tendermint is tendermint/tendermint#6375.

Closes: #XXX

@torao torao force-pushed the feature/enable_libsodium branch 4 times, most recently from 97863c5 to cbf9c6b Compare August 4, 2021 09:27
@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #297 (dc1734f) into main (f53ee28) will increase coverage by 1.64%.
The diff coverage is 68.20%.

❗ Current head dc1734f differs from pull request most recent head a1ef7de. Consider uploading reports for the commit a1ef7de to get more accurate results

@@            Coverage Diff             @@
##             main     #297      +/-   ##
==========================================
+ Coverage   61.18%   62.82%   +1.64%     
==========================================
  Files         272      272              
  Lines       29988    30171     +183     
==========================================
+ Hits        18347    18954     +607     
+ Misses       9963     9484     -479     
- Partials     1678     1733      +55     
Impacted Files Coverage Δ
abci/types/application.go 0.00% <0.00%> (ø)
abci/types/messages.go 16.05% <0.00%> (-1.55%) ⬇️
blockchain/v2/processor_context.go 63.15% <0.00%> (ø)
blockchain/v2/reactor.go 34.91% <ø> (ø)
cmd/ostracon/commands/light.go 15.82% <0.00%> (ø)
cmd/ostracon/commands/root.go 46.87% <0.00%> (ø)
config/toml.go 68.00% <ø> (ø)
consensus/replay_file.go 0.00% <0.00%> (ø)
consensus/replay_stubs.go 60.00% <0.00%> (+42.05%) ⬆️
crypto/bls/bls.go 48.41% <ø> (ø)
... and 94 more

@torao torao force-pushed the feature/enable_libsodium branch 3 times, most recently from eed25ef to e648277 Compare August 4, 2021 11:02
@torao torao self-assigned this Aug 5, 2021
@torao torao marked this pull request as draft August 5, 2021 06:13
@torao torao force-pushed the feature/enable_libsodium branch 8 times, most recently from 5fedfa0 to 104e7f5 Compare August 12, 2021 05:25
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale For github bot label Aug 23, 2021
@torao torao force-pushed the feature/enable_libsodium branch 4 times, most recently from d8783bf to f45f549 Compare August 25, 2021 05:45
@github-actions github-actions bot closed this Aug 30, 2021
@torao torao reopened this Sep 1, 2021
@torao torao added C: enhancement Classification: New feature or its request, or improvement in maintainability of code and removed Stale For github bot labels Sep 1, 2021
@torao torao force-pushed the feature/enable_libsodium branch from f45f549 to 554a413 Compare September 1, 2021 05:33
@torao torao changed the title WIP: Change VRF Implementation to Libsodium Support for VRF implementation with libsodium Sep 1, 2021
@torao torao marked this pull request as ready for review September 1, 2021 05:38
@torao torao requested review from tnasu and Kynea0b September 1, 2021 05:39
Copy link
Member

@tnasu tnasu left a comment

Choose a reason for hiding this comment

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

I'm not sure that the changing of submodules: recursive help to make the next PR of default libsodium. For example, should we add new tasks like build-linux task on .github/workflows/coverage.yml? And should it depends on Makefile tasks since it's easy to build with libsodium?

tests.mk Outdated
.PHONY: test

test_race:
@echo "--> Running go test --race"
@echo "--> Running go test -- race"
Copy link
Member

Choose a reason for hiding this comment

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

How about adding -tags ${BUILD_TAGS}

Kynea0b
Kynea0b previously approved these changes Sep 1, 2021
Copy link
Contributor

@Kynea0b Kynea0b left a comment

Choose a reason for hiding this comment

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

LGTM

@torao torao force-pushed the feature/enable_libsodium branch from 2203744 to e7e3a2a Compare September 2, 2021 04:15
@torao torao force-pushed the feature/enable_libsodium branch from dc1734f to a1ef7de Compare September 2, 2021 11:27
Copy link
Member

@tnasu tnasu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Kynea0b Kynea0b left a comment

Choose a reason for hiding this comment

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

LGTM

@torao torao merged commit d86ec62 into Finschia:main Sep 3, 2021
tnasu added a commit that referenced this pull request Jan 6, 2022
* rpc/jsonrpc: Unmarshal RPCRequest correctly (bp #6191) (#6193)

* rpc/jsonrpc: Unmarshal RPCRequest correctly (#6191)

i.e. without double pointer. With double pointer, it was possible to
submit `null` value, which will crash the server.

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x189ddc0]

goroutine 1 [running]:
github.com/tendermint/tendermint/rpc/jsonrpc/types.(*RPCRequest).UnmarshalJSON(0xc0000147e0, 0xc00029f201, 0x4, 0x1ff, 0x883baa0, 0xc0000147e0)
        /Users/anton/go/src/github.com/tendermint/tendermint/rpc/jsonrpc/types/types.go:70 +0x100
encoding/json.(*decodeState).literalStore(0xc000216bb0, 0xc00029f201, 0x4, 0x1ff, 0x1998800, 0xc0000147e0, 0x199, 0xc000231700, 0x10e0a5e, 0x197)
        /usr/local/Cellar/go/1.16/libexec/src/encoding/json/decode.go:860 +0x30ce
encoding/json.(*decodeState).value(0xc000216bb0, 0x1998800, 0xc0000147e0, 0x199, 0x1998800, 0xc0000147e0)
        /usr/local/Cellar/go/1.16/libexec/src/encoding/json/decode.go:384 +0x40c
encoding/json.(*decodeState).array(0xc000216bb0, 0x18df040, 0xc0001be540, 0x16, 0xc000216bd8, 0x10e405b)
        /usr/local/Cellar/go/1.16/libexec/src/encoding/json/decode.go:558 +0x365
encoding/json.(*decodeState).value(0xc000216bb0, 0x18df040, 0xc0001be540, 0x16, 0x16, 0x6e)
        /usr/local/Cellar/go/1.16/libexec/src/encoding/json/decode.go:360 +0x22f
encoding/json.(*decodeState).unmarshal(0xc000216bb0, 0x18df040, 0xc0001be540, 0xc000216bd8, 0x0)
        /usr/local/Cellar/go/1.16/libexec/src/encoding/json/decode.go:180 +0x2c9
encoding/json.Unmarshal(0xc00029f200, 0x6, 0x200, 0x18df040, 0xc0001be540, 0x0, 0x0)
        /usr/local/Cellar/go/1.16/libexec/src/encoding/json/decode.go:107 +0x15d
```

(cherry picked from commit fe4e97a)

* fix conflict

Co-authored-by: Anton Kaliaev <[email protected]>

* Fix codecov for `rpc/jsonrpc: Unmarshal RPCRequest correctly (bp #6191) (#6193)`

* logs: cleanup (#6198)

Co-authored-by: Marko <[email protected]>

* Fix codecov for `logs: cleanup (#6198)`

* mempool/rpc: log grooming (bp #6201) (#6203)

* indexer: remove info log (#6194)

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Marko <[email protected]>

* e2e: add benchmarking functionality (bp #6210) (#6216)

* note: add nondeterministic note to events (#6220) (#6225)

Since events are not hashed into the header they can be non deterministic. Changing an event is not consensus breaking. Will update docs in the spec

(cherry picked from commit 884d4d525299e4d43ab881ac19062501c1e09ddf)

Co-authored-by: Marko <[email protected]>

* use error.Is to check for nondeterminstic vote error type (#6237) (#6239)

(cherry picked from commit bf8cce83db15ee2645924799a4b2dfba260788dc)

Co-authored-by: Callum Waters <[email protected]>

* rpc/jsonrpc/server: return an error in WriteRPCResponseHTTP(Error) (bp #6204) (#6230)

* rpc/jsonrpc/server: return an error in WriteRPCResponseHTTP(Error) (#6204)

instead of panicking
Closes #5529

(cherry picked from commit 00b952416836cb568ee904903d30b35a6178bad1)

* resolve conflicts

* fix linting

* fix conflict

Co-authored-by: Anton Kaliaev <[email protected]>
Co-authored-by: Marko Baricevic <[email protected]>

* Fix codecov for `rpc/jsonrpc/server: return an error in WriteRPCResponseHTTP(Error) (bp #6204) (#6230)`

* e2e: integrate light clients (bp #6196)

integrate light clients (#6196)
fix e2e app test (#6223)
fix light client generator (#6236)

* Fix lint for `e2e: integrate light clients (bp #6196)`

* Fix codecov for `e2e: integrate light clients (bp #6196)`

* rpc: index block events to support block event queries (bp #6226) (#6261)

* Fix codecov for `rpc: index block events to support block event queries (bp #6226) (#6261)`

* logging: shorten precommit log message (#6270) (#6274)

This is an attempt to clean up the logging message as requested in #6269.

(cherry picked from commit 3f9066b290ba2de266b42aa6ad2b076c7be4f5df)

Co-authored-by: Sam Kleinman <[email protected]>

* fix: jsonrpc url parsing and dial function (#6264) (#6288)

This PR fixes how the jsonrpc parses the URL, and how the dial function connects to the RPC.

Closes: tendermint/tendermint#6260

(cherry picked from commit 9ecfcc93a6364bbecc9e1d7740b53602eda667eb)

Co-authored-by: Frojdi Dymylja <[email protected]>

* change index block log to info (#6290) (#6294)

Change log from error to info for indexing blocks

(cherry picked from commit 32ee737d42d3092dec6c99729927483abc1bd959)

Co-authored-by: Marko <[email protected]>

* p2p: Fix "Unknown Channel" bug on CustomReactors (#6297)

* state: fix block event indexing reserved key check (#6314) (#6315)

* light/evidence: handle FLA backport (#6331)

* Revert evidence/pool.go in `Support for VRF implementation with libsodium (#297)`

* Fix test for `light/evidence: handle FLA backport (#6331)`

* Ignore test for `light/evidence: handle FLA backport (#6331)`

* Imporve the execution time of light test

* Improve log format

* Bugfix logging value

* Refactor function for usability

* Fix `TestLightClientAttackEvidence_Lunatic`

* Fix `TestLightClientAttackEvidence_Equivocation`

* Fix `TestLightClientAttackEvidence_ForwardLunatic`

Co-authored-by: Anton Kaliaev <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Callum Waters <[email protected]>
Co-authored-by: Sam Kleinman <[email protected]>
Co-authored-by: Frojdi Dymylja <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: enhancement Classification: New feature or its request, or improvement in maintainability of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants