search

thibault-martinez / iota.lib.cpp

IOTA C++ Library
MIT License
79 stars 21 forks source link

isPromotable & checkConsistency #310

Closed Cylix closed 1 year ago

Cylix commented 6 years ago

From #304 (iota.lib.js changes):

isPromotable(tail) of develop branch is now renamed to checkConsistency(tail)
isPromotable(tail, depth) is now expressed as function of checkConsistency(tail) & depth, because promotion becomes ineffective after a few milestones. Internally uses a time heuristic: https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createIsPromotable.ts

Seems we have both isPromotable and checkConsistency, should we dismiss isPromotable?

Cylix commented 6 years ago

For this one, was a bit confused by the wording, but after investigation, seems that what we actually need to do is to support a new parameter depth for isPromotable.

The implementation can be found here.

chrisdukakis commented 6 years ago

Hey @Cylix & @thibault-martinez ,

most likely we are removing isPromotable() and keep checkConsistency() without depth argument & isAboveMaxDepth() predicate.

I think IRI checks depth now: https://github.com/iotaledger/iri/blob/bed15673709a033bd32029d6e567b032ec7d5bd8/src/main/java/com/iota/iri/service/API.java#L461 https://github.com/iotaledger/iri/blob/1829df5dde8dadbc78f482d4b7db43ca0a934015/src/main/java/com/iota/iri/service/tipselection/impl/WalkValidatorImpl.java#L85

If I understand it correctly, pure checkConsistency as seen here is sufficient. I'll create an issue in iota.js & link you to it, as soon as I've confirmed that's the case.

Also if you use isPromotable as the name of the call, it would be better to rename it to checkConsistency, to match with IRI api.