In the Dart CLI the htlc.create command has an optional hashlock parameter. If it’s provided it’ll use that as the hashlock and won’t generate a preimage.
Thanks for the context, wasn’t aware of this. Looks like it’s best to add an optional KeyMaxSize param to the htlc.create command in the Dart CLI as well.
Right, got it. I’ll update the C# CLI accordingly and remove the incompatibility issues.
I’ll update this, but I think we should also check the length of the hashLock.
The variables htlcPreimageMinLength (32) and htlcPreimageMaxLength (32) aren’t enforced in either znn_sdk_dart or go-zenon. I was able to create a 99-byte preimage and unlock an HTLC with it.
Any suggestions for lower/upper boundaries or should we enforce 32/32?
I don’t see the problem with the check the length of the hashLock.
The hashLock length is determined by the output size of the hashType. Both hash types supported use a 256 bit output size. The hashLock is not valid if it does not equal 32 bytes in length.
How the preimage is generated is determined by the implementation of the calling application. I’m not sure what the benefits would be by enforcing its length.
The Dart CLI htlc.unlock command uses an extra hashType argument to do a preimage check. Wouldn’t it be easier to just use the hashType already available in the retrieved htlc object?
Right, no issues in go-zenon. I should have specified – I’d like to add matching client-side input validation in znn_cli_dart and syrius.
We probably don’t want to allow weak preimages; I’d imagine some actors will try to bruteforce active swaps.
I guess we don’t need to enforce the limit in the protocol (go-zenon) but it could help to do so in order to align all SDK/client applications with the same rule.
Yes, it would be easier but I found that is error-prone. If a user submits a pre-generated SHA-256 hashlock but forgets to enter the type in htlc.create, their type will default to 0. This optional flag in hltc.unlock allows them to recover the funds without having to wait for the swap to expire.
Doesn’t this already happen when creating the hash object from the Zenon Sdk which indirectly enforces the length of the hashLock. The parse and fromBytes methods perform a validation on the length.
Whether weak preimages are used depends on the client implementation. It makes sense to make recommendations on how to generate preimages. Also users of x client need to know that the preimages the client generates are considered secure. This should prevent weak preimages from being created, unless one creates them directly using the sdk.
Enforcing the limit in the client won’t stop someone from bruteforce active swaps. Again, one could easily use the sdk directly.
I guess if it makes sense to enforce the limits it has to be done in the protocol.
I see, good thinking.
I checked the code again; you’re probably right and doing a client-side length check seems redundant.
Just want to be sure we’re all on the same page.
The protocol and SDKs will not have any preimage length restrictions.
Clients like znn-cli and syrius will impose min/max preimage lengths.
What should the min/max be? I was assuming 32/32 until yesterday.
We’re on the same page 
I personally don’t think the limits add much, but if we add them 32/32 seems like the most sensible limit.
Let me get back on this one. The htlc unlock contract creates a SHA3-256 hashlock of the supplied preimage which will never match the SHA-256 hashlock. So I fail to see how someone could recover the funds without having to wait for the swap to expire.
// Alice creates a SHA-256 hash
znn-cli createHash 1
Preimage: c770b3160b83f6e3a7e45e961da8aaafbdc1557bb2f4dee3ae16d370e10fe16d
Hash: 3b6e7bfafa188dff9f50b12be1143c751653453a27097cd297f2f283e7e804f3
// Alice creates a htlc with a pre-generated SHA-256 hashlock, but forgets to specify the hashtype
znn-cli htlc.create z1qpsjv3wzzuuzdudg7tf6uhvr6sk4ag8me42ua4 ZNN 100 3600 3b6e7bfafa188dff9f50b12be1143c751653453a27097cd297f2f283e7e804f3 -k Alice
Creating htlc with amount 100,00000000 tZNN
Can be reclaimed in 01:00:00 by z1qqjnwjjpnue8xmmpanz6csze6tcmtzzdtfsww7
Can be unlocked by z1qpsjv3wzzuuzdudg7tf6uhvr6sk4ag8me42ua4 with hashlock 3b6e7bfafa188dff9f50b12be1143c751653453a27097cd297f2f283e7e804f3 hashtype 0
Done
// Bob checks the hash locked htlc's
znn-cli htlc.hashLocked -k Bob
Htlc id b1fdbd1888a689538587d273b18778bc26ae2dc8387669da8bad29c80ead4cb0 with amount 100,00000000 tZNN
Can be reclaimed in 00:59:24 by z1qqjnwjjpnue8xmmpanz6csze6tcmtzzdtfsww7
Can be unlocked by z1qpsjv3wzzuuzdudg7tf6uhvr6sk4ag8me42ua4 with hashlock 3b6e7bfafa188dff9f50b12be1143c751653453a27097cd297f2f283e7e804f3 hashtype 0
// Bob unlocks the htlc and specifies the correct hash type.
znn-cli htlc.unlock b1fdbd1888a689538587d273b18778bc26ae2dc8387669da8bad29c80ead4cb0 c770b3160b83f6e3a7e45e961da8aaafbdc1557bb2f4dee3ae16d370e10fe16d 1 -k Bob
Unlocking htlc id b1fdbd1888a689538587d273b18778bc26ae2dc8387669da8bad29c80ead4cb0 with amount 100,00000000 tZNN
Done
Use receiveAll to collect your htlc amount after 2 momentums
// Bob checks the hash locked htlc's and notices the htlc is not unlocked by the contract
znn-cli htlc.hashLocked -k Bob
Htlc id b1fdbd1888a689538587d273b18778bc26ae2dc8387669da8bad29c80ead4cb0 with amount 100,00000000 tZNN
Can be reclaimed in 00:57:20 by z1qqjnwjjpnue8xmmpanz6csze6tcmtzzdtfsww7
Can be unlocked by z1qpsjv3wzzuuzdudg7tf6uhvr6sk4ag8me42ua4 with hashlock 3b6e7bfafa188dff9f50b12be1143c751653453a27097cd297f2f283e7e804f3 hashtype 0
Where are these variables?
The HTLC embedded contract stores the KeyMaxSize variable in a uint8 so the max preimage length the protocol supports is 255 bytes. I think that should be enforced by clients. There’s no min preimage length and I’m not sure if there’s much benefit in enforcing one client-side.
Was this with Dart CLI or C# CLI?
In go-zenon and znn_sdk_dart.
What’s your ideal default user experience for znn-cli and syrius?
Should znn-cli permit 0/255, default 32?
And should syrius present similar options?
I tried both
Also, I get the following error when using the verbose flag.
Unhandled exception:
Unsupported operation: Please set "hierarchicalLoggingEnabled" to true if you want to change the level on a non-root logger.
#0 Logger.level= (package:logging/src/logger.dart:123)
#1 initZnn (file:///d:/repos/sol-sanctum/znn_cli_dart/init_znn.dart:137)
#2 main (file:///d:/repos/sol-sanctum/znn_cli_dart/cli_handler.dart:17)
#3 _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:295)
#4 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:192)
I’ve pushed some changes to my repo.
The verbose parameter should work now, though I haven’t tested this thoroughly.
As for the scenario you pointed out, you’re right. It won’t work.
I’ve adjusted the htlc.unlock function accordingly.
znn-cli: default keyMaxSize = 32, with the possibility of calling createHash to generate a preimage of [1, 255] bytes.
Great work, but there’s still a problem with the htlc.create function.
The keyMaxSize is always 32, even when supplying a hashlock based on a pre-image with a larger size.
Thinking about the keyMaxSize. I think it’s best to always set it to 255. The only reason it’s there is because of the Secret Size Attacks. Settings it to the max allowed pre-image size, prevents the attacks and allows the creation of pre-images between 1 and 255, using a default of 32.
The following line in the createHash function needs to be an or statement; otherwise the condition is never met.
if (keyMaxSize > htlcPreimageMaxLength && keyMaxSize < 1) {
Just updated the following work:
That’s embarassing 
Should be fixed by these commits: SDK and CLI
I think I’m done making changes now.
I think you brought down my devnet with your spork test. 
It’s back up now.
Great, looks good to me 
Yeah, I thought that might be the case . That means the activation worked 
I will try to be more carefull next time 
I guess the CLI can permit anything between 0-255 but for Syrius I’m not sure. For ZTS <> ZTS swaps the 32 byte preimage should be fine and not something the user needs to care about but for cross-chain swaps the length might have to be specified by the user.
Maybe I missed something but what is the purpose of these variables in go-zenon?
I don’t think they’re being used in go-zenon.