Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(release): v1.0.6-beta #1871

Merged
merged 45 commits into from
Jul 24, 2023
Merged

chore(release): v1.0.6-beta #1871

merged 45 commits into from
Jul 24, 2023

Conversation

shamardy
Copy link
Collaborator

@shamardy shamardy commented Jun 20, 2023

Features:

  • Swap watcher nodes #1431
    • Using watcher nodes for swaps were enabled by default for UTXO coins in #1859
      • use_watchers configuration was set to true by default. It was later disabled in #1897 due to this issue #1887
      • All nodes doing a swap will broadcast a watcher message after the taker payment is sent if the swapped coins are supported by watchers (currently only UTXO). This was also disabled in #1897 due to this issue #1887
      • This update also fixes an issue that caused nodes to broadcast two consecutive watcher messages after the taker payment was sent.
  • NFT integration #900
    • Cache support was added for sqlite (non-wasm targets) in #1833
    • IndexedDb support for wasm was added in #1877
    • DB unit tests were added in #1877
    • Handling of bafy in IPFS Moralis links in a correct way was done in #1877
    • get_uri_meta function was added to optimize the retrieval of UriMeta from token_uri and metadata in #1877
    • protect_from_spam feature was added to redact URLs in specific fields and flag them as possible spam in #1877
  • HTTPS support was added for the RPC server in #1861
  • Adex-CLI #1682
    • New commands enable, get-enabled, orderbook,sell, buy were added to adex-cli in #1768

Enhancements/Fixes:

  • Some RUSTSEC advisories where resolved in #1853
  • ARRR/ZCOIN code was refactored to be compiled in WebAssembly (WASM) in #1805
    • The PR for this paves the way for subsequent implementation of the empty/todo functions related to WASM storage and other functionalities.
  • Orderbook response now returns the right age for the age field, this was fixed in #1851
  • A bug that caused best_orders rpc to return is_mine: false for the user's orders was fixed in #1846
    • An optional parameter exclude_mine was also added to the best_orders request that allows users to exclude their own orders from the response.
    • exclude_mine defaults to false to maintain the same behaviour before the PR.
  • Watchtower integration tests were moved to the new ethereum testnet and the ignore attributes were removed in #1846
    • The PR also adds a new test case for watcher rewards.
    • It also fixes the unstable send_and_refund_eth_payment, send_and_refund_erc20_payment, test_nonce_lock and test_withdraw_and_send tests tests that were failing due to concurrency issues.
  • Infrastructure DNS rotation for default seednodes was done in #1868
  • Price endpoints were updated in #1869
  • A fix removed the passed config string from the error logs during mm2 initialization if there was a deserialization error was done in #1872
  • The time needed for CI completion was reduced by caching the downloaded dependencies in #1880
  • Label validation on PRs was added. This validation will only succeed if one of the following labels is used but not both: under review or in progress #1881
  • orderbook mod of adex-cli was refactored by moving it from the internal response_handler to its appropriate folder, enhancing code organization and clarity in #1879
  • A bug was fixed for adex-cli to allow starting if configuration does not exist in #1889
  • IBC and standard withdrawals for Cosmos now allow users to specify the gas price and gas limit for each transaction #1894
  • A fix was introduced to adex-cli to allow starting mm2 from cli under regular user in macOS #1856

onur-ozkan and others added 16 commits June 9, 2023 02:04
This commit refactors ARRR/ZCOIN code to be compiled in WebAssembly (WASM). It paves the way for subsequent implementation of the empty/todo functions related to WASM storage and other functionalities.

---------
Signed-off-by: borngraced <samuelonoja970@gmail.com>
When orderbook is requested the age field now returns the right age.
Signed-off-by: ozkanonur <work@onurozkan.dev>
…ption (#1849)

This commit fixes a bug that caused `best_orders` rpc to return is_mine: false for the user's orders. It also adds an optional `exclude_mine` parameter to the `best_orders` request that allows users to exclude their own orders from the response. `exclude_mine` defaults to false to maintain the same behaviour before this commit.
* Moves the watchtower integration tests to the new ethereum testnet and remove the ignore attributes.
* Adds a new test case for watcher rewards.
* Fixe the unstable `send_and_refund_eth_payment`, `send_and_refund_erc20_payment`, `test_nonce_lock` and `test_withdraw_and_send tests` that were failing due to concurrency
This commit changes the komodo endpoint for fetching swap prices to a working one
This commit makes `use_watchers` configuration `true` by default. This means that all nodes will broadcast a watcher message after the taker payment is sent if the swapped coins are supported by watchers (only UTXO for now). It also fixes a problem that caused the nodes to broadcast two watcher messages consecutively after the taker payment is sent.
- add the required deps for native_tls to mm2_net/Cargo.toml
- expose TlsStream and add remote_addr to it
- format the ported code
…1768)

This commit introduces the following new commands to adex-cli. `enable`, `get-enabled`, `orderbook`,`sell`, `buy`
This commit adds NFT cache support for sqlite while leaving wasm IndexedDB implementation as Todo since the logic that uses sqlite NFT cache is applicable to both targets.
This commit removes the passed config string to mm2 while initializing from the error log if there was a deserialization error.
@shamardy shamardy marked this pull request as ready for review June 21, 2023 22:04
@shamardy
Copy link
Collaborator Author

shamardy commented Jun 21, 2023

@ca333 @Alrighttt @DeckerSU dependency updates or new dependencies can be found in #1861 (comment), #1768 (comment), #1853 (comment) or just check the below tables instead.

mm2 Dependency Updates:

Updated:

librustzcash k-1.0.0 -> k-1.3.0
blake2 v0.10.4 -> v0.10.6
metrics v0.19.0 -> v0.21.0
hyper v0.14.18 -> v0.14.26
rusqlite v0.24.2 -> v0.28.0
env_logger v0.9.0 -> v0.9.3
ahash 0.7.6 -> 0.8.3
block-modes 0.7.0 -> 0.8.1
fpe 0.3.13 -> 0.3.19
hashbrown 0.12.1 -> 0.13.2
hashlink 0.6.0 -> 0.8.2
httparse 1.6.0 -> 1.8.0
libsqlite3-sys 0.20.1 -> 0.25.2
metrics-exporter-prometheus 0.10.0 -> 0.12.1
metrics-macros 0.5.1 -> 0.7.0
metrics-util 0.13.0 -> 0.15.0
num-traits 0.2.12 -> 0.2.15
ordered-float 2.10.0 -> 3.7.0
pkg-config 0.3.17 -> 0.3.27
quanta 0.9.3 -> 0.11.1
rmp 0.8.9 -> 0.8.11
sketches-ddsketch 0.1.3 -> 0.2.1
socket2 0.4.4 -> 0.4.9
termcolor 1.1.0 -> 1.2.0
version_check 0.9.2 -> 0.9.4
rustls-pemfile v1.0.0 -> v1.0.2
base64 v0.13.0 -> v0.21.2

New:

Instead of
libm v0.2.7
mach2 v0.4.1 mach v0.3.2
portable-atomic v1.3.2
pem v1.1.1
rcgen v0.10.0
yasna v0.5.2

adex-cli Dependency Updates:

Updated:

mm2 Cargo.lock
clap 2.34.0 -> 4.3.4 2.33.3
hashbrown 0.12.3 -> 0.14.0 0.11.2,0.12.1,0.13.2
hashlink 0.6.0 -> 0.8.3 0.8.2
strsim 0.8.0 -> 0.10.0 0.8.0
wasm-bindgen 0.2.80 -> 0.2.87 0.2.86
wasm-bindgen-backend 0.2.80 -> 0.2.87 0.2.86
wasm-bindgen-macro 0.2.80 -> 0.2.87 0.2.86
wasm-bindgen-macro-support 0.2.80 -> 0.2.87 0.2.86
wasm-bindgen-shared 0.2.80 -> 0.2.87 0.2.86

New:

mm2 Cargo.lock
allocator-api2 0.2.15
anstream 0.3.2
anstyle 1.0.0
anstyle-parse 0.2.0
anstyle-query 1.0.0
anstyle-wincon 1.0.1
arrayvec 0.5.2 0.5.1,0.7.1
base64 0.13.1 0.10.1,0.11.0, 0.12.3, 0.13.0, 0.21.2
bigdecimal 0.3.1 0.3.0
blake2b_simd 0.5.11 0.5.10
clap_builder 4.3.4
clap_derive 4.3.2
colorchoice 1.0.0
directories 5.0.1 3.0.2
dirs-sys 0.4.1 0.3.6
errno 0.3.1 0.2.8
heck 0.4.1 0.4.0
hermit-abi 0.3.1 0.1.14
io-lifetimes 1.0.11 1.0.6
is-terminal 0.4.7
keccak 0.1.4 0.1.0
linux-raw-sys 0.3.8 0.1.4
num-rational 0.4.1 0.4.0
option-ext 0.2.0
paste 1.0.12 1.0.7
portable-atomic 1.3.3 1.3.2
redox_users 0.4.3 0.3.4, 0.4.0
rustix 0.37.20 0.36.9
sct 0.6.1 0.6.0,0.7.0
utf8parse 0.2.1
webpki 0.21.4 0.21.3,0.22.0

Thank you @rozhkovdmitrii for the table format.

@DeckerSU
Copy link
Collaborator

DeckerSU commented Jun 22, 2023

Btw, JFYI ... there is a cargo plugin named cargo-lockdiff to generate similar diff tables. For example, for our case it generates the following, as a diff between Cargo.lock in v1.0.6 and v1.0.5:

Package From To Review Result / Compare
aes-soft 0.6.4 REMOVED 🟢
aesni 0.10.0 REMOVED 🟢
ahash NEW 0.8.3 🟢 0.7.6 -> 0.8.3
atomic-shim 0.2.0 REMOVED 🟢
base-x 0.2.8 REMOVED 🟢
base64 NEW 0.21.2 🟢 0.13.0 -> 0.21.2
blake2 0.10.4 0.10.6 🟢 ...
block-modes 0.7.0 0.8.1 🟢 ...
const_fn 0.4.7 REMOVED 🟢
discard 1.0.4 REMOVED 🟢
env_logger 0.9.0 0.9.3 🟢 ...
fpe 0.4.0 0.5.1 🟢 ...
h2 0.3.13 0.3.19 🟢 ...
hashbrown NEW 0.13.2 🟢 0.12.1 -> 0.13.2
hashlink 0.6.0 0.8.2 🟢 ...
httparse 1.6.0 1.8.0 🟢 ...
hyper 0.14.18 0.14.26 🟢 ...
indexmap 1.7.0 1.9.3 🟢 ...
libm NEW 0.2.7 🟢 (only for use ceil and log2 in fpe)
libsqlite3-sys 0.20.1 0.25.2 🟢... (sqlite3 and sqlcipher amalgamation C sources compared with sqlite-amalgamation-3390200.zip and v4.5.2.tar.gz official sources accordingly)
mach 0.3.2 REMOVED 🟢
mach2 NEW 0.4.1 🟢 (almost the same as earlier used mach 0.3.2)
metrics 0.19.0 0.21.0 🟢 ...
metrics-exporter-prometheus 0.10.0 0.12.1 🟢 ...
metrics-macros 0.5.1 0.7.0 🟢 ...
metrics-util 0.13.0 0.15.0 🟢 ...
num-traits 0.2.12 0.2.15 🟢 ...
ordered-float 2.10.0 3.7.0 🟢 ...
pem NEW 1.1.1 🟢
petgraph 0.6.1 0.6.3 🟢 ...
pkg-config 0.3.17 0.3.27 🟢 ...
portable-atomic NEW 1.3.2 🟢
proc-macro-hack 0.5.19 REMOVED 🟢
quanta 0.9.3 0.11.1 🟢 ...
rcgen NEW 0.10.0 🟢
rmp 0.8.9 0.8.11 🟢 ...
rmp-serde 0.14.3 0.14.4 🟢 ...
rusqlite 0.24.2 0.28.0 🟢 ...
rustls-pemfile 1.0.0 1.0.2 🟢 ...
serde 1.0.136 1.0.164 🟢 ...
serde_derive 1.0.136 1.0.164 🟢 ...
sha1 0.6.0 REMOVED 🟢
sketches-ddsketch 0.1.3 0.2.1 🟢 ... (used in metrics)
socket2 0.4.4 0.4.9 🟢 ...
standback 0.2.17 REMOVED 🟢
stdweb 0.4.20 REMOVED 🟢
stdweb-derive 0.5.3 REMOVED 🟢
stdweb-internal-macros 0.2.9 REMOVED 🟢
stdweb-internal-runtime 0.1.5 REMOVED 🟢
termcolor 1.1.0 1.2.0 🟢 ...
time-macros-impl 0.1.2 REMOVED 🟢
version_check 0.9.2 0.9.4 🟢 ...
yasna NEW 0.5.2 🟢

It's good, but it's not ideal, like it was unable to determine changes in librustzcash, etc.

p.s. I will mark the reviewed packages in Review Result column with 🟢, 🟡, 🔴 symbols and corresponding comments.

Copy link
Collaborator

@DeckerSU DeckerSU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A review of changed dependencies, including librustzcash, has been done. Everything seems to be okay. I also want to note that the review was only carried out on "malicious inclusions" and does not affect errors in the logic of the operation of certain modules. Obviously, it is impossible to know the nuances of how all crates work, such as, for example, the distributed quantile sketch algorithm, etc. At this moment, the mm2 sources themselves have not been reviewed; I will do it a bit later and leave a comment here.

@shamardy
Copy link
Collaborator Author

@cipig UTXO watchers should be tried and tested before approving and merging this PR. You can refer to this document until this feature is fully documented. You can reach out to me or @caglaryucekaya if you have any questions about how to set this up and what to expect from it.

This commit reduces the time needed for CI by caching the downloaded dependencies.

---------

Signed-off-by: ozkanonur <work@onurozkan.dev>
@shamardy
Copy link
Collaborator Author

shamardy commented Jun 26, 2023

Merged #1880, #1881 since they don't add or change anything in deps and only related to CI.

This commit adds label validation on PRs. The validation will only succeed if one of the following labels is used but not both: `under review` or `in progress`.

---------

Signed-off-by: ozkanonur <work@onurozkan.dev>
@tonymorony tonymorony added the blocked requires additional attention label Jul 11, 2023
@@ -1070,19 +1075,19 @@ impl TakerSwap {
};

// This will be done during order match
self.w().watcher_reward = false;
self.w().watcher_reward = std::env::var("USE_WATCHER_REWARD").is_ok();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid reading a system variable like this throughout the execution of the program. Ideally it should be read only once while initializing mm2 or possibly while enabling specific coins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anarkiVan
Copy link

After switching to dev there are some problems/regression with updated NFT functionality vs current (1.0.5) release version . Komodo Wallet devs currently isolating/preparing report [ CC @anarkiVan @laruh ] So it's better to not merge this PR until it works smoothly/without komodowallet team / QA approval on NFT feature test, otherwise GUI will be blocked

#1909

sql_builder
.sql_builder()
.and_where_eq("token_address", format!("'{}'", token_address))
.and_where_eq("token_id", format!("'{}'", token_id))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function has potential for sql injection if the moralis endpoint were to somehow return malicious token_address values or if it were ever reused with a different source for the token_id argument.

eg,
let token_id = "a_token_id'; DROP TABLE an_important_table;'".to_string();
or
let token_address = "a_token_address'; DROP TABLE an_important_table;'".to_string();

We should be deserializing any data from external sources to strict types. For example, if we know token_address must be an EVM compatible address, we should deserialize it directly to a Address type and convert to String as needed to ensure we are in fact dealing with a valid address.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use prepared statement here @laruh

We should avoid using sql_builder as it will be replaced or refactored to be built on prepared statements. Any query created with sql_builder is vulnerable to SQL injections.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, on it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here

Signed-off-by: ozkanonur <work@onurozkan.dev>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include a license file here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the licences for the ported mod https://github.com/rustls/hyper-rustls/tree/286e1fa57ff5cac99994fab355f91c3454d6d83d#license, I think we should include them. Will do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be included in this file instead https://github.com/KomodoPlatform/komodo-defi-framework/blob/dev/LEGAL/THIRDPARTY-LICENSES . I don't know a lot about licencing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that hyper-rustls is already in our thirdparty licences file here

Apache-2.0 OR ISC OR MIT (4): ct-logs, hyper-rustls, rustls, sct

@ca333 should anything else be added because we ported some of hyper-rustls new code and changed it a bit to fit what we needed.

## Purpose

The purpose of porting these files is to enable retrieving the remote address from the incoming connection and to expose the `TlsStream` type.
> **Note:** The following commit [7eca34d](https://github.com/KomodoPlatform/atomicDEX-API/pull/1861/commits/7eca34dd4621a7de0033f8a81cc11ad117aeb3c3) show the changes applied to the ported code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a broken link.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out, will fix it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here dd1b098

Alrighttt
Alrighttt previously approved these changes Jul 13, 2023
Copy link
Member

@Alrighttt Alrighttt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all 3rd party dependency changes. Reviewed all our code changes. I don't consider any of the above issues blocking other than the sql injection concerns. Ready to merge at @shamardy's discretion.

* run all tests ignoring the failed ones

Signed-off-by: ozkanonur <work@onurozkan.dev>

* Update adex-cli.yml

---------

Signed-off-by: ozkanonur <work@onurozkan.dev>
- Address was used instead of string in NFT and transaction objects
- `guard: Arc<AsyncMutex<()>>` from struct `NftCtx` is added to lock nft functions which uses db
- IndexedDB Cursor collect method is used to fix uncaught Error
@shamardy
Copy link
Collaborator Author

After switching to dev there are some problems/regression with updated NFT functionality vs current (1.0.5) release version .
Komodo Wallet devs currently isolating/preparing report
So it's better to not merge this PR until it works smoothly/without komodowallet team / QA approval on NFT feature test, otherwise GUI will be blocked

These issues should be solved by this commit 1e0cf5b, @tonymorony please remove blocked label when everything is fine.

shamardy and others added 2 commits July 18, 2023 03:43
)

This commit addresses index out of bounds errors in the `tx_details_by_hash` functions. The changes replace direct array access with safer methods, avoiding potential panics due to out-of-range indices. In addition, error handling around the function was improved to log and skip transactions in case of errors.
@tonymorony tonymorony removed the blocked requires additional attention label Jul 22, 2023
@tonymorony
Copy link

@shamardy the NFT related issues were resolved, removed the label

smk762
smk762 previously approved these changes Jul 24, 2023
Copy link

@smk762 smk762 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work

@shamardy shamardy merged commit ef89614 into main Jul 24, 2023
reddink pushed a commit to reddink/komodo-defi-framework that referenced this pull request Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.