test(wallet): create tx locktime cltv for a specific time #1682
Conversation
f3r10
changed the title
crates/wallet/src/test_utils.rs
Outdated
| /// the parameter passed to miniscript fragment `after` has to equal or grather than 500_000_000 | ||
| /// in order to use a lock based on unix time | ||
| pub fn get_test_single_sig_cltv_for_specific_time() -> &'static str { | ||
| "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(500000000)))" |
There was a problem hiding this comment.
⛏️ just a nit but we should use a timestamp that's sometime after the genesis block. Sometimes people use these tests as examples so will make more sense to use a more recent time. Also the number is easier to read in the code with the underscores.
| "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(500000000)))" | |
| "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(1_734_230_218)))" |
There was a problem hiding this comment.
I tried to update the descriptor as you mentioned, but there is an error with the format:
descriptors must be valid: Miniscript(Unexpected("unexpected «1_734_230_218»"))
It looks like miniscript does not have support for numbers with underscores.
crates/wallet/src/test_utils.rs
Outdated
| @@ -160,6 +160,13 @@ pub fn get_test_single_sig_cltv() -> &'static str { | |||
| "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(100000)))" | |||
| } | |||
|
|
|||
| /// `wsh` descriptor with policy `and(pk(A),after(500_000_000))` | |||
| /// the parameter passed to miniscript fragment `after` has to equal or grather than 500_000_000 | |||
There was a problem hiding this comment.
| /// the parameter passed to miniscript fragment `after` has to equal or grather than 500_000_000 | |
| /// the parameter passed to miniscript fragment `after` has to equal or greater than 500_000_000 |
There was a problem hiding this comment.
@f3r10 I don't think the extra commentary is needed, but we can make it a code comment as a note to developers and leave the doc string as a single line:
/// `wsh` descriptor with policy `and(pk(A),after(1734230218))`
Can you give this helper a better name, like get_test_single_sig_cltv_timestamp. I agree with the suggestion to use a realistic timestamp (without underscores).
There was a problem hiding this comment.
@ValuedMammal fixed with your comments.
However, based on what we talked about at the previous meeting, I was about to close this PR.
Do you think that this test is testing what is supposed to?
Maybe when the implementation of the planning module that you are working on is finished, this test will be more relevant. 🤔
There was a problem hiding this comment.
yes I think we should have the test, I'm not sure of another way to test that time based timelocks are satisfied.
f3r10
force-pushed
the
test_timelock_descriptors
branch
from
a636a6b to
af30026
Compare
Description
This PR adds a unit test that checks the satisfaction of timestamp-based timelocks. The goal is to test the absolute time therefore the variable passed to the miniscript fragment
afterhas to be equal to or greater that 500_000_000 otherwise it would be checking the block height.Notes to the reviewers
This unit test tries to check if #642 is still an issue.
Checklists
All Submissions:
cargo fmtandcargo clippybefore committing