Fix event decoding in cli + remove timeout in demos #507
Conversation
haerdib
changed the title
haerdib
changed the title
clangenb
left a comment
clangenb
left a comment
There was a problem hiding this comment.
Looks good, although I am not 100% convinced that his actually fixes the issue.
Can you convince me? :)
| let ret: ProcessedParentchainBlockArgs = _chain_api | ||
| .wait_for_event::<ProcessedParentchainBlockArgs>( |
There was a problem hiding this comment.
So here we waited for BlockConfirmed args. Did block confirm change over time? If not, I am not 100% convinced that this solves the issue.
There was a problem hiding this comment.
I'm honestly not 100% sure what the previous problem was, but I think it might have been partially fixed by cleaning up the Confirmations on the worker side th PR #501 and now fixing the Block listening event... But the tests pass now without a Timeout. Is that not convincing enough?
There was a problem hiding this comment.
If they pass consistently, I am convinced. :D
| $TIMEOUT ${CLIENT} trusted set-balance ${ICGACCOUNTALICE} ${AMOUNTSHIELD} --mrenclave ${MRENCLAVE} --direct | ||
| ${CLIENT} trusted set-balance ${ICGACCOUNTALICE} ${AMOUNTSHIELD} --mrenclave ${MRENCLAVE} --direct |
There was a problem hiding this comment.
Removing the timeout means that we potentially wait forever? Or what was the motivation?
There was a problem hiding this comment.
Yes, that is true. But since this is used as a CI test, I think we should know if no fitting confirmation is issued by the parentchain.
There was a problem hiding this comment.
So my motiviation: Wait forever such that we know something is wrong.
| struct ProposedSidechainBlockArgs { | ||
| struct ProcessedParentchainBlockArgs { |
There was a problem hiding this comment.
Small but significant change in how to read this code 😄
murerfel
left a comment
murerfel
left a comment
There was a problem hiding this comment.
LGTM - just some minor questions for my own understanding
fixes #456