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

rpcsrv: carefully store Oracle service instance #3085

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

AnnaShaleva
Copy link
Member

@AnnaShaleva AnnaShaleva commented Aug 9, 2023

Problem

Invalid submitoracleresponse RPC handler behaviour in case of disabled Oracle service: no neorpc.NewRPCError("Oracle is not enabled", "") error is returned. Thanks to @tatiana-nspcc for the report.

Solution

Check for typed nil value while setting Oracle service. I know, that we don't want to import the whole oracle package from the rpcsrv, but the alternatives I see looks worser to me:

  1. Check that Oracle service is a proper nil value each time when rpcsrv.New(...) is used. It's not very convinient; there are three places in the code and a couple more in tests.
  2. Use reflection inside rpcsrv to ensure that Oracle service is really nil. We'd better avoid reflection untill there are no other choices.

@AnnaShaleva AnnaShaleva added bug Something isn't working oracle Oracle service and contract labels Aug 9, 2023
@AnnaShaleva AnnaShaleva added this to the v0.102.0 milestone Aug 9, 2023
Broken by #3084.

Signed-off-by: Anna Shaleva <[email protected]>
@AnnaShaleva
Copy link
Member Author

The test for the case of disabled Oracle service is added in the #3063.

@roman-khimov
Copy link
Member

This makes OracleHandler interface completely useless. I'd just have pythia rpcsrv.OracleHandler in the code calling rpcsrv.New (just three real cases and one is always a proper nil) and initialize it accordingly (mkOracle is almost ready to return rpcsrv.OracleHandler as well, maybe an additional interface would solve it).

cli/server/server.go Outdated Show resolved Hide resolved
pkg/services/rpcsrv/server.go Outdated Show resolved Hide resolved
And simplify atomic service value stored by RPC server. Oracle service can
either be an untyped nil or be the proper non-nil *oracle.Oracle.
Otherwise `submitoracleresponse` RPC handler doesn't work properly.

Signed-off-by: Anna Shaleva <[email protected]>
Comment on lines -2464 to +2466
oracle := s.oracle.Load().(*OracleHandler)
if oracle == nil || *oracle == nil {
oraclePtr := s.oracle.Load()
if oraclePtr == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! Thank you for fixing

@roman-khimov roman-khimov merged commit 2493400 into master Aug 10, 2023
11 of 16 checks passed
@roman-khimov roman-khimov deleted the fix-oracle-instance branch August 10, 2023 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working oracle Oracle service and contract
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants