feat: add btp version as extra argument

interledgerjs / btp-packet

Packet parser for Bilateral Transfer Protocol

1 stars 3 forks source link

feat: add btp version as extra argument #16

Closed michielbdejong closed 7 years ago

michielbdejong commented 7 years ago

The version called version 1 on https://github.com/interledger/interledger/wiki/Interledger-over-CLP (aka the testnet version) is called version 'alpha' here, and the version 2 from https://github.com/interledger/interledger/wiki/Interledger-over-CLP#changes-between-versions-1-and-2 (aka the freeze version) will from now on be called version 1.

sharafian commented 7 years ago

I don't think we should worry about supporting the alpha version, because BTP wasn't finalized at the time. Once we have the spec freeze on monday, we should say that's the first supported version.

michielbdejong commented 7 years ago

@emschwartz and I discussed this in Slack: we could say the testnet version needs no version number because ‘nobody is using it’, and the freeze version then also needs no version number because ‘everybody is using it’. but that just means we had a choice between introducing a versioned protocol and an unversioned one, and we chose the unversioned one.

michielbdejong commented 7 years ago

This issue kept me up last night, and I think I know now where the confusion came from. Let me break it down into 4 questions:

should this module be usable in the 'community network' (aka 'the open interledger')? -> yes
which versions of BTP should be used there? -> only version BTP/1.0 for now
should this module be usable in the Interledger Testnet of Testnets -> yes
which versions of BTP should be used there? ->that is the open question, which we can discuss in https://github.com/michielbdejong/ilp-node/issues/46

Depending on the outcome of how the testnet should work, we can still say testnet nodes should include btp-packet twice (once for btp alpha support and once for btp 1 support), but I think that discussion would then be easier to reach a conclusion on.

michielbdejong commented 7 years ago

I need to keep alpha-version support for the 17Q3 endpoint on Amundsen. Adding it in this module will make it easier to implement that in a clean way. I set it to default to version 1, so it doesn't affect other users of this module.

michielbdejong commented 7 years ago

Thanks!!