Skip to content

Conversation

@pinheadmz
Copy link
Member

Closes #516
Resolves a5cc393

There are several tests that make "fake" reserved name claims in order to test the HNS reserved name claim system. That system looks up actual, real-world, ICANN-rooted DNS records and checks the DNSSEC chain. The only thing "fake" about the tests is that once we download the actual live DNSSEC chain, the test inserts a wallet claim such as this into the cloudflare.com zone:

cloudflare.com. 300 IN TXT "hns-regtest:aakbvmygsp7rrhmsauhwlnwx6srd5m2v4m3p3eidadl5yn2f"

Of course this invalidates the RRSIG TXT in that zone, but we have a bypass just for this precise case so we can test everything else:

try {
ownership.ignore = true;
assert(await chain.add(block));
} finally {
ownership.ignore = false;
}

The issue is that now the test suite is dependent on the runner's ability to access the internet, and lookup the cloudflare.com DNS zone. I think that is OK -- since the ability to do so is also crucial for the HNS reserved name claims system to work.

What this PR fixes is a set value that the fake claim redeems by claiming cloudflare on regtest. The claim still pays a fee, which is an amount of HNS based on the size of the claim (just like any other type of transaction). However, cloudflare is welcome to change their DNS records at any time -- add or remove TXT in their zone, sign RRSIGs with different key algorithms, etc. This means we can not predetermine the size of the fake claim for the test and therefore can not assert that a specific value of HNS will be redeemed by submitting the claim on regtest.

Solution: set the fee rate to zero :-)

@pinheadmz pinheadmz requested review from chjj and tynes September 22, 2020 16:47
@pinheadmz pinheadmz added this to the v2.3.0 milestone Sep 22, 2020
@coveralls
Copy link

coveralls commented Sep 22, 2020

Pull Request Test Coverage Report for Build 267283005

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 5 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.1%) to 59.367%

Files with Coverage Reduction New Missed Lines %
lib/net/pool.js 1 32.63%
lib/net/peer.js 2 35.28%
lib/wallet/wallet.js 2 70.0%
Totals Coverage Status
Change from base Build 238010293: 0.1%
Covered Lines: 19423
Relevant Lines: 30471

💛 - Coveralls

@ca98am79
Copy link
Contributor

Tested ACK - works for me

@pinheadmz pinheadmz merged commit ff54a15 into handshake-org:master Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants