status-im/nim-stew

Conflict between stew.init and blsCurve.init

Opened this issue · 5 comments

It seems like the init scheme here is a too general (with auto) and causes issue with bls curve init from status-im/nim-blscurve#26

template init*(lvalue: var auto) =
mixin init
lvalue = init(type(lvalue))
template init*(lvalue: var auto, a1: auto)=
mixin init
lvalue = init(type(lvalue), a1)
template init*(lvalue: var auto, a1, a2: auto) =
mixin init
lvalue = init(type(lvalue), a1, a2)
template init*(lvalue: var auto, a1, a2, a3: auto) =
mixin init
lvalue = init(type(lvalue), a1, a2, a3)

proc init*[T: SigKey|VerKey|Signature](obj: var T,
                                       data: openarray[byte]): bool {.inline.} =
  ## Initialize ``SignatureKey``, ``VerificationKey`` or ``Signature`` from
  ## raw binary representation ``data``.
  ##
  ## Procedure returns ``true`` on success and ``false`` otherwise.
  when T is SigKey:
    result = obj.x.fromBytes(data)
  else:
    result = obj.point.fromBytes(data)

The error thrown is especially hard to debug

../beacon_chain/spec/crypto.nim(195, 7) template/generic instantiation from here
../beacon_chain/spec/crypto.nim(186, 18) template/generic instantiation from here
../beacon_chain/spec/crypto.nim(125, 21) Error: expression '
buffer = init(type(buffer), a.blob)' has no type (or is ambiguous)

corresponding to this line:

  let success = buffer.init(a.blob)

just to check, this is not an ambiguity between the init added in crypto.nim in the interop branch?

zah commented

The error message is not that confusing to me, but the question is why Nim selected the overload from stew. My expectation is that it should either select the init from bls-curve because it's more specific or that you should get an overload resolution error on the init call itself. (the current error is that the selected overload from stew returns void and you cannot assign that to success).

Are you sure both modules were imported in the context where init was instantiated? Don't forget about the generics "sandwich problem". You must have the correct overload visible in the outermost module where the chain of generic instantiations starts.

@arnetheduck no, it's in master branch

@zah
Yes I'm sure it's visible. both auto and T have the same level of specificity AFAIK (generics).

For now I can workaround with blscurve.init.

By the way, which of our libraries rely on those stew/inits?

zah commented

Nim counts the number of auto matches in the signature though and the init from stew should have more of them.

I'm trying to use these overloads whereever they are applicable because they have been proposed for inclusion in the standard library. Our codebase should serve as a proof that they are reasonable.

well, what's not nice about them (as we saw in the bls case) is that they still are not force-called like a constructor - so it's much better to design types that don't need them rather than using init and hoping that they'll be called.. their value thus goes down significantly.