Add error handling to Ethereum lookups

[buffrr]

There are many cases in which Infura requests could fail causing the plugin to crash. If the plugin fails, hsd returns the NS record with the Ethereum contract to the recursive resolver which leads to SERVFAIL for future lookups.

Example:

dig @127.0.0.1 -p 5350 doesntexist.badass
dig @127.0.0.1 -p 5350 certified.badass

This PR checks if the resolver address returned from registry is empty the doesntexist.badass case but also wraps Ethereum lookups with try/catch and logs the errors to prevent any intermittent errors from Infura from causing the plugin to crash.

[pinheadmz]

I actually for some reason didn't have an issue with this (see below) although I agree a try/catch is needed for infura calls.

(main branch):

--> git rev-parse head
a8b57eae41f9b17a8ab0faacc7fafbf6844758a6

--> dig @127.0.0.1 -p 5350 doesntexist.badass

; <<>> DiG 9.16.10 <<>> @127.0.0.1 -p 5350 doesntexist.badass
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: SERVFAIL, id: 29600
;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 2

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 4096
; COOKIE: fea2487bfcc750aa (echoed)
;; QUESTION SECTION:
;doesntexist.badass.            IN      A

;; SIG0 PSEUDOSECTION:
.                       0       ANY     SIG     0 253 0 0 20210714002005 20210713122005 15350 . /DORk0qAypdD3AlIh+GyZNaT7lS+J7NsgDUfFi1JaRpeEoK1vhmZElcR KLuNDRHUQxk13fz94mMxJjC/XYCy6w==

;; Query time: 390 msec
;; SERVER: 127.0.0.1#5350(127.0.0.1)
;; WHEN: Tue Jul 13 14:20:05 EDT 2021
;; MSG SIZE  rcvd: 153

--> dig @127.0.0.1 -p 5350 certified.badass

; <<>> DiG 9.16.10 <<>> @127.0.0.1 -p 5350 certified.badass
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 58577
;; flags: qr rd ra ad; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 2

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 4096
; COOKIE: fc8de08737d3cd24 (echoed)
;; QUESTION SECTION:
;certified.badass.              IN      A

;; ANSWER SECTION:
certified.badass.       60000   IN      A       184.73.82.1

;; SIG0 PSEUDOSECTION:
.                       0       ANY     SIG     0 253 0 0 20210714002009 20210713122009 15350 . nQ9UK/jWJhn38cOPcTQx5fLDNogbupEcx73ugDG/7mAT0szBZRFGz+Fz dcqG/86+RivQR+n3g+F5+eeLH9nRfw==

;; Query time: 655 msec
;; SERVER: 127.0.0.1#5350(127.0.0.1)
;; WHEN: Tue Jul 13 14:20:09 EDT 2021
;; MSG SIZE  rcvd: 167

definitely had an error:

[warning] (ns) Root server middleware resolution failed for name: doesntexist.badass.
[debug] (ns) Error: call revert exception (method="dnsRecord(bytes32,bytes32,uint16)", errorSignature=null, errorArgs=[null], reason=null, code=CALL_EXCEPTION, version=abi/5.0.12)
    at Logger.makeError (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/logger/lib/index.js:179:21)
    at Logger.throwError (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/logger/lib/index.js:188:20)
    at Interface.decodeFunctionResult (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/abi/lib/interface.js:286:23)
    at Contract.<anonymous> (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/contracts/lib/index.js:319:56)
    at step (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/contracts/lib/index.js:46:23)
    at Object.next (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/contracts/lib/index.js:27:53)
    at fulfilled (/Users/matthewzipkin/Desktop/work/hsd/node_modules/handover/node_modules/@ethersproject/contracts/lib/index.js:18:58)

[buffrr]

I actually for some reason didn't have an issue with this (see below) although I agree a try/catch is needed for infura calls.

You may have to try a few times sometimes it works on the first attempt because recursive may have not cached the NS record. Once the recursive sees the NS record and caches it, it will SERVFAIL for other lookups.

Maybe to reproduce this more reliably you can try this :)

dig @127.0.0.1 -p 5350 doesntexist.badass 
dig @127.0.0.1 -p 5350 doesntexist00.badass 
dig @127.0.0.1 -p 5350 doesntexist01.badass
dig @127.0.0.1 -p 5350 certified.badass

[pinheadmz]

Maybe to reproduce this more reliably you can try this :)

great got it thanks. SO I definitely approve of this change, just a few code style questions.

ALSO -- I think when i started this i was thinking "if the name doesnt resolve on ethereum correctly, DO NOTHING" which is why the ...._eth NS record comes back. But of course you are right about how cache works and we can't do that. So that being said, I think it would be awesome if you opened a PR to HIP-0005 explaining that software implementing that protocol SHOULD simply return null (or NX, whatever it ends up being) instead of throwing an error, and we can explicitly add that the NS records should not be returned so we dont end up poisoning cache.

[pinheadmz]

One small nit (remove outdated comment) but otherwise this is great thanks!