We should assume that Alice always has access to the Redis instance to lock with. Bob's lock-service should ping Alice's to lock the channel.

Bob should be able to deposit by sending a tx to the channelAddress. Right now the protocol breaks when this is done, it should default to reading the address balance in case the channel isn't deployed.

Initiator creates, responder resolves

Multidevice bb -- need to be able to restore from backup for all channels within the protocol

This test should pass:

it.skip("should work concurrently", async () => {});

• prod deployment (ability to run node and router-node on separate machines)
• ability set node mnemonic during runtime
• hashlock-transfer timelock
• hashlock-status endpoint
  • needs expired: true | false field
• cancel transfer endpoint
• onchain balances
• expose ethprovider proxy
• request collateral

Should search for either Alice or Bob combos.

Listing down all the issues encountered after a general walk-through of the readme:

1. make start-trio gets stuck at

   Preparing to launch trio stack
   
   Preparing to launch trio stack
   global.auth configured to be exposed on *:5040
   WARNING: generating new nats jwt signing keys & saving them in .env
   

   was fixed by manually populating the .env

2. Nomenclature and Port numbers mismatch on the make start-trio command, the README and the scripts inside /modules/server-node/examples

   • the fix is required in the scripts and the README to match with the output from make command

   •     trio.carol will be exposed on *:8005
         trio.dave will be exposed on *:8006
         trio.roger will be exposed on *:8007
         trio.router will be exposed on *:8009
     

3. The 3-transfer.http script mentions transfer API as /linked-transfer/, should instead be /hashlock-transfer/

4. For the README:

   • the purpose of linkedHash and routingId
   • mention of the withdraw API (POST {{bobUrl}}/withdraw) in the README
   • viewing logs (not sure if it should be mentioned in the README, but would be helpful) bash ops/logs.sh carol
   • maybe a mention of what a hashlock transfer is, and why it requires resolution of transfers for the recipient to be able to claim

We need to do some thinking here around the best way to handle sending to chain. Should we just dispatch to onchainService no matter what and then if the service doesn't have funds or keys then it just returns null?

Expected Behavior

The engine should not allow unresolvable (or invalid) transfers to be created in its channel. There should be some injected validation to ensure this. Blocked by #25

Current Behavior

Always creates a transfer application (regardless of transfer state).

Full update validation for initiators and responders

Notes:

- Need to be able to run start-router and start-node on separate machines. Right now they each spin up their own global stack.
- For testing, we tried to run them on the same machine.
- Both start-node and start-router use the same mnemonic.
- We tried to change mnemonics in the `start-node` script but it still didn't change mnemonics. This is likely because the secrets and/or db volumes are not namespaced properly.

We should not restrict this at the contract level

Look into using: https://github.com/a7ul/esbuild-node-tsc

Should look very similar to or almost exactly the same as the server-node, except with a TS interface + indexedDB store

Transfers should be allowed to go outside the channel, represented via a different address in the balance.to entry. This address would receive funds in the event of a dispute.

Currently, the engine hardcodes the transfer recipient (for linked transfers) to be the channel counterparty instead of a recovery address. The protocol also assumes that any balance headed out of the channel within a transfer does not get reconciled back into the channel balance on resolution

Context:

While we're redesigning the transfers anyway, this might be a good time to actually take the initial balances out of the encoded transfer state, in order to avoid having that information stored redundantly in the core transfer state, which is a potential source of errors.
[...] What I think is not ideal with the current solution is that the initial balance is stored redundantly in the core transfer: in the initialBalance and (hashed) in the initialStateHash. The former is used in the adjudicator, the latter is used by the transfer definition -- and we hope we don't make a mistake that gets the two out of sync... Admittedly, that's not very likely, because it's the "initial" state and therefore not supposed to change. Still, having the supposedly same information doubly-stored in the transfer state is not really clean.
That's why I wanted to suggest to change the signature of the create and resolve function to take in the initial balance as (unencoded) first argument and take it completely out of the (encoded) app-specific state. For example, the LinkedTransfer would then only have the linkedHash as the app-specific state.

The BrowserNode imports & uses ServerNodeParams & ServerNodeResponses; to keep naming scope correct, they should be changed to something more generic (and anything else that is named for the server and used in both, if they exist)

Ideally, implementers should be able to pass in their own custom conditional transfer definitions without needing to make changes to our types. This can be done (unsafely) by allowing users to pass in transferDef instead of transferType + restructuring conditionalTransfer params to look more like the transfer initial state.

To do this safely, we need to implement a registry pattern where we have a global onchain registry of "approved" transferDefs which users can call/reference. That way, routers can keep blindly forwarding transfers without needing to worry about whether the transfer definition might be malicious.

Recommend splitting the above into two separate PRs, with the first paragraph happening immediately.

Expected Behavior

The following tests should pass:

describe("generateSignedChannelCommitment", () => {
    it("should not sign anything if there are two signatures", () => {});
    it("should work for participants[0] if there is not a counterparty signature included", () => {});
    it("should work for participants[1] if there is not a counterparty signature included", () => {});
    it("should work for participants[0] if there is a counterparty signature included", () => {});
    it("should work for participants[1] if there is a counterparty signature included", () => {});
});

describe("validateChannelUpdateSignatures", () => {
    it("should work for a valid single signed update", () => {});
    it("should work for a valid double signed update", () => {});
    it("should fail if there are not at the number of required sigs included", () => {});
    it("should fail if any of the signatures are invalid", () => {});
});

Problem

Can't run the happy case duet or trio tests multiple times in a row (will fail if channel is setup between the same participants)

Solution

Create snapshots of the chain to restore tests too in the integration test setups

This is a prereq for multihop that can be knocked out in 30 mins - ping me directly for details.

Expected Behavior

Return Result and handle it properly

Current Behavior

Returns a promise

Currently:

• LinkedTransfer resolves based on a hash preImage & optionally accepts a recipient who's allowed to resolve
• HashlockTransfer resolves based on a hash preImage & enforces a timeout
• SignedTransfer resolves based on a signature w/out a timeout

I propose:

• HashlockTransfer resolves based on a hash preImage
• SignedTransfer resolves based on a signature
• both accept an expiry after which the transfer gets reverted, 0 means it never expires

Core transfer params would be: (type: "SignedTransfer" | "HashlockTransfer", payload: NonceToSign | HashedPreImage, timeout: number)

SignedTransfer would become createTransfer("SignedTransfer", nonce, 0)
HashlockTransfer would become createTransfer("HashlockTransfer", hash, timeout)
LinkedTransfer w recipient specified would become createTransfer("SignedTransfer", nonce, 0)
LinkedTransfer w/out a recipient would become createTransfer("HashlockTransfer", hash, 0)

We could consider pulling the expiry enforcement logic into the adjudicator too to prevent it from being duplicated across every app definition.

Should be passing:

it.skip("should work for alice creating transfer to bob", async () => {});
  it.skip("should work for alice creating transfer out of channel", async () => {});
  it.skip("should work for bob creating transfer to alice", async () => {});
  it.skip("should work for bob creating transfer out of channel", async () => {});
  it.skip("should work if channel is out of sync", async () => {});

Encryption and decryption of resolver, resolved on start-up, etc.

The Problem

At the protocol, there are some details you are unable to validate based on the information you have available, including:

• validity of the networkContext in setup updates
• validity of the transferEncodings[1] (resolver encoding) in create updates before signing a transfer into your channel

Potential Solution

Validate this via the validation service the protocol is instantiated with on the engine. It is a bit strange, since they are core to the protocol, but the engine will have to validate that the transfer encodings match the transfer definitions anyway so I don't think it is too bad.

Protocol should be aware of disputes to the point that it no longer continues updating the channel when it is in dispute (and there is the ability to manually resolve it)

Expected Behavior

The following generateUpdate tests should be implemented:

it("should fail if it fails parameter validation", () => {});
it("should fail if it is unable to reconcile the deposit", () => {});
it("should fail if trying to resolve an inactive transfer", () => {});
it("should fail if fails to call resolve using chain service", () => {});
it("should work if creating a transfer to someone outside of channel", () => {});
it("should work if resolving a transfer to someone outside of channel", () => {});

Describe the bug
Multiple setup calls throw a strange error.

To Reproduce

1. Call setup endpoint in example .http file from start-duet mode.

Expected behavior
Should be a more clear error.

Alice side:

[1600845344685] INFO  (493 on 7656c9ff4880): Sending protocol message
    module: "VectorProtocol"
    method: "outbound"
    to: "indra5ArRsL26avPNyfvJd2qMAppsEVeJv11n31ex542T9gCd5B1cP3"
    type: "setup"
[1600845344703] INFO  (493 on 7656c9ff4880): Received protocol response
    module: "VectorProtocol"
    method: "outbound"
    to: "indra5ArRsL26avPNyfvJd2qMAppsEVeJv11n31ex542T9gCd5B1cP3"
    type: "setup"
[1600845344706] ERROR (493 on 7656c9ff4880):
    message: "Cannot read property 'signatures' of undefined"
    stack: "TypeError: Cannot read property 'signatures' of undefined\n    at Object.<anonymous> (/root/modules/protocol/src/sync.ts:142:42)\n    at Generator.next (<anonymous>)\n    at fulfilled (/root/modules/protocol/dist/sync.js:5:58)\n    at processTicksAndRejections (internal/process/task_queues.js:97:5)"

Bob side:

[1600845344690] INFO  (497 on bb62da2c5b35): Received message
    module: "VectorProtocol"
    method: "onReceiveProtocolMessage"
[1600845344699] ERROR (497 on bb62da2c5b35): Error validating incoming channel update
    module: "VectorProtocol"
    method: "inbound"
    channel: "0xc27e351f43a8cCad42B609343a0A43905336c97b"
    error: "Update does not progress channel nonce"
[1600845344700] ERROR (497 on bb62da2c5b35): Error validating update
    module: "VectorProtocol"
    method: "inbound"
    error: "Update does not progress channel nonce"

Server node:

HTTP/1.1 500 Internal Server Error
content-type: application/json; charset=utf-8
content-length: 60
Date: Wed, 23 Sep 2020 08:20:54 GMT
Connection: close

{
  "message": "Cannot read property 'signatures' of undefined"
}

setup on the server node should actually call an endpoint on the counterparty's server node to have them set up a channel with you.

The following functions should return Result types in the utils file within the protocol module:

1. validateChannelUpdateSignatures
2. generateSignedChannelCommitment

Expected Behavior

Should not sign withdrawal resolution updates if there is a fee and the transaction has not been submitted to chain (or the submitted transaction is invalid)

When a protocol user comes online, it should send an isAlive message to all channel counterparts so they know when to send queued updates

https://github.com/connext/vector/tree/master/modules/router

The readme here references collateral in several contexts, as well as "executing collateralization", but the term is not defined, nor its function and purpose defined. Anything that a router operator should know about re:collateral should probably also be documented.

Combine transactions into single deploy + deposit tx.

These tests should pass:

it.skip("should deposit tokens for alice", async () => {});
  it.skip("should deposit tokens for bob", async () => {});
  it.skip("should work after multiple deposits", async () => {});
  it.skip("should work if there have been no deposits onchain", async () => {});
  it.skip("should work if the channel is out of sync", async () => {});
  it.skip("should work if channel is out of sync", async () => {});

Expected Behavior

The following inbound tests should be implemented:

it.skip("should fail if you are 3+ states behind the update", async () => {});
  it.skip("should fail if validating the update fails", async () => {});
  it.skip("should fail if saving the data fails", async () => {});
  it.skip("IFF update is invalid and channel is out of sync, should fail on retry, but sync properly", async () => {});
  describe.skip("should sync channel and retry update IFF state nonce is behind by 2 updates", async () => {
    describe.skip("initiator trying deposit", () => {
      it("missed setup, should work", async () => {});
      it("missed deposit, should work", async () => {});
      it("missed create, should work", async () => {});
      it("missed resolve, should work", async () => {});
    });

    describe.skip("initiator trying create", () => {
      it("missed deposit, should work", async () => {});
      it("missed create, should work", async () => {});
      it("missed resolve, should work", async () => {});
    });

    describe.skip("initiator trying resolve", () => {
      it("missed deposit, should work", async () => {});
      it("missed create, should work", async () => {});
      it("missed resolve, should work", async () => {});
    });
  });

The following outbound tests should be implemented:

it.skip("should fail if update to sync is single signed", async () => {});
  it.skip("should fail if the channel is not saved to store", async () => {});
  it.skip("IFF update is invalid and channel is out of sync, should fail on retry, but sync properly", async () => {});
describe("should sync channel and retry update IFF update nonce === state nonce", async () => {
    describe.skip("initiator trying setup", () => {
      it("missed setup, should sync without retrying", async () => {});
    });

    describe("initiator trying deposit", () => {
      it("missed deposit, should work", async () => {})
      it.skip("missed create, should work", async () => {});
      it.skip("missed resolve, should work", async () => {});
    });

    describe.skip("initiator trying create", () => {
      it("missed deposit, should work", async () => {});
      it("missed create, should work", async () => {});
      it("missed resolve, should work", async () => {});
    });

    describe.skip("initiator trying resolve", () => {
      it("missed deposit, should work", async () => {});
      it("missed create, should work", async () => {});
      it("missed resolve, should work", async () => {});
    });
  });

These tests should pass:

it.skip("should work for withdraw", async () => {});
  it.skip("should work for alice resolving an eth transfer", async () => {});
  it.skip("should work for alice resolving an eth transfer out of channel", async () => {});
  it.skip("should work for alice resolving a token transfer", async () => {});
  it.skip("should work for alice resolving a token transfer out of channel", async () => {});
  it.skip("should work for bob resolving an eth transfer", async () => {});
  it.skip("should work for bob resolving an eth transfer out of channel", async () => {});
  it.skip("should work for bob resolving a token transfer", async () => {});
  it.skip("should work for bob resolving a token transfer out of channel", async () => {});
  it.skip("should work if channel is out of sync", async () => {});

The onchain service does not retry transactions automatically, or store sent transactions permanently. This means in the event of a server crash, any pending transactions will be lost (though the commitments will be saved)

Bot-farm-like load tests that use the new server-node key management infra to create many many transfers and stress test the router

Expected Behavior

Protocol does not have context into transfer-specific validation, but the engine should be able to inject some.

Current Behavior

Only runs provided validation (i.e. without transfer context)