Polkadex-Substrate / polkadexTEE-worker

Polkadex Off-chain Orderbook
Apache License 2.0
10 stars 1 forks source link

incorporate upstream changes into polkadexTEE-worker #304

Open haerdib opened 2 years ago

haerdib commented 2 years ago

Integritee-worker is being continuously upgraded. Some changes are unrelevant for the polkadexTEE-worker, but some are certainly helpfull improvements.

In this issue we should discuss how we should handle upgrading to the upstream branch. The following questions should be answered: 1) Do we even want to follow suit of the integritee-worker?

2) If yes: a) Should we rather cherry pick commits that are useful for us, and leave out others? (with the danger of more merge conflicts later on) or b) should we just sometimes update & merge to the most recent upstream master?

The problems I see are the following: Option a) : we will have to check every single commit & review the cherry-pick as a PR again in the polkadexTEE worker. We would also need to decide who should decide on which commits should be integrated and which not. Option b): That will create a huge PR, in case we wait for a little bit longer, increasing the chance of introducing some errors, especially if we don't have integration test that fully cover all use cases of the worker.

haerdib commented 2 years ago

As decided in the Roadmap call (18.10.2021) I will summarize what changes have been done upstream and are useful for polkdex. We can then decide together if we want to integrate them or not based on this summary.

haerdib commented 2 years ago

I'm recommending to integrate the following commits (starting from 23th of august, PR #288 already includes older updates)

General comment: All these extractions & abstractions to extra crates actually help to make the whole codebase better unit testable. A separate crate can be easily tested with cargo test and included in enclave with no-std import when needed in there. (cargo test not available inside encalve, making it hard to unit test)

What we do not need to include:

haerdib commented 2 years ago

@simonsso is the above list somewhat sensible to you? And if yes, can you agree to the separation I made?

simonsso commented 2 years ago

see ticked boxes

haerdib commented 2 years ago

TODO include: