Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix registrar errors early in the chain #11257

Merged
merged 1 commit into from
Nov 15, 2019

Conversation

dvdplm
Copy link
Collaborator

@dvdplm dvdplm commented Nov 13, 2019

In a previous PR the registrar contract was overhauled. After it, syncing from the beginning of the chain spews out a ton of errors like this:

2019-11-13 15:23:13  Verifier #9 ERROR client  Error occurred while refreshing service transaction cache: please ensure the contract and method you're calling exist! failed to decode empty bytes. if you're using jsonrpc this is likely due to jsonrpc returning `0x` in case contract or method don't exist

It looks a bit nasty but I don't think it's critical. At some point in the chain the problem goes away (not sure exactly were tbh but sometime after ~3M blocks).

The error message comes from ethabi and what it really means is we passed in an empty slice of bytes to the decode_output(). Further up in the chain we get 0x0000000000000000000000000000000000000000 back and so we hit the address.is_zero() case and all is good (I have a DB synced to ~4.4M where I see this).

So my question in this PR is: is it ok to change the code to interpret [] as "a zero address" and return Ok(None) here?

@dvdplm dvdplm changed the title Fix registrart errors early in the chain Fix registrar errors early in the chain Nov 13, 2019
@dvdplm dvdplm self-assigned this Nov 13, 2019
@dvdplm dvdplm added the A0-pleasereview 🤓 Pull request needs code review. label Nov 13, 2019
@dvdplm dvdplm requested a review from tomusdrw November 13, 2019 16:01
@dvdplm dvdplm added A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B1-patch-beta 🕷🕷 M4-core ⛓ Core client code / Rust. labels Nov 13, 2019
Copy link
Collaborator

@debris debris left a comment

Choose a reason for hiding this comment

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

So my question in this PR is: is it ok to change the code to interpret [] as "a zero address" and return Ok(None) here?

I think this is a correct assumption. lgtm

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 15, 2019
@tomusdrw
Copy link
Collaborator

Yeah, it's most likely missing contract code if it returns an empty bytes vector.

@ordian ordian added this to the 2.7 milestone Nov 15, 2019
@debris debris merged commit 93f700d into master Nov 15, 2019
@niklasad1 niklasad1 deleted the dp/fix/spurious-registrar-errors-early-in-the-chain branch November 15, 2019 15:56
dvdplm added a commit that referenced this pull request Nov 20, 2019
* master:
  Clarify what first_block `None` means (#11269)
  removed redundant VMType enum with one variant (#11266)
  Ensure jsonrpc threading settings are sane (#11267)
  Return Ok(None) when the registrar contract returns empty slice (#11257)
  Add a benchmark for snapshot::account::to_fat_rlps() (#11185)
  Fix misc compile warnings (#11258)
  simplify verification (#11249)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants