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

chain-test: cover CSV by time #613

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pinheadmz
Copy link
Member

This PR adds tests to check CHECKSEQUENCEVERIFY functionality using the time parameter as well as the height parameter. I'm also not entirely sure the existing tests were totally right. They had CSV OP codes in the outputs of the spending transaction, which is unnecessary, and the fail csv with bad sequence test wasn't actually spending from a CSV output.

I wrote this PR mainly as an exploration of CSV for a cross-chain atomic swap module. The utility function CSVencode is necessary to check against time instead of block height (see BIP68) and this is the function I really wanted to test -- if you want to use time-based CSV in a transaction you'll need to encode the time this way and I don't think there's currently a method anywhere in the library that does this? So actually it might make more sense to put that method somewhere else, like in chain, mtx, or script?

@codecov-io
Copy link

codecov-io commented Oct 4, 2018

Codecov Report

Merging #613 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #613      +/-   ##
==========================================
+ Coverage   53.32%   53.37%   +0.05%     
==========================================
  Files         104      104              
  Lines       27666    27666              
  Branches     4737     4737              
==========================================
+ Hits        14752    14766      +14     
+ Misses      12914    12900      -14
Impacted Files Coverage Δ
lib/primitives/tx.js 81.51% <0%> (-0.1%) ⬇️
lib/primitives/mtx.js 60.88% <0%> (+0.34%) ⬆️
lib/blockchain/chain.js 61.38% <0%> (+0.83%) ⬆️
lib/script/opcode.js 78.92% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 125e818...865864a. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Has tests for code change is just an addtional test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants