Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl

[Mimi Zohar <zohar@xxxxxxxxxxxxx>]

On Thu, 2020-04-02 at 10:19 +0300, Vitaly Chikunov wrote:
> > > +
> > > +  # Test --portable
> > > +  expect_pass check_sign OPTS="$OPTS --portable" PREF=0x05
> > > +  # Cannot be verified
> > 
> > True, evmctl does not support verifying portable signatures, but it
> > should be possible not only locally, but remotely to verify a portable
> > signature.  That's the whole point of having portable EVM signatures.
> >  The comment is a bit misleading and could say something to that
> > effect - "todo: add support for evmctl portable signature
> > verification".
> 
> Well, tests are not right place to note todos for other code.
> This todo would look like we need to add test case to the test, like
> test is missing something. While now it says that it impossible to test.
> 
> I will change text to something like "Cannot be verified for now, until
> that support is added to evmctl".

Thanks.

While you're updating the "sign_verify.test", could you expand this
comment a bit?  Referencing an example would help.

# Fix keyid in the prefix.
  if [[ $PREF =~ K ]]; then
    keyid=$(_keyid $KEY)

thanks,

Mimi