Skip to content

deposit and withdraw#12

Closed
Siddharth2207 wants to merge 37 commits intomainfrom
2023-07-28-ob-cli
Closed

deposit and withdraw#12
Siddharth2207 wants to merge 37 commits intomainfrom
2023-07-28-ob-cli

Conversation

@Siddharth2207
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

missing documentation and tests for almost everything here

Comment thread cli/Cargo.toml Outdated
spinners = "4.1.0"
hex = "0.4.3"
ethers-signers = { version = "2.0.8", features = ["ledger"] }
tui = "0.19"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove if we're not using

Comment thread cli/src/cli/addorder/addorder.rs Outdated
let tokens = tokens ;
let decimals = decimals ;

let vault_id = U256::from(H160::random().as_bytes()) ;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this needs to be an input otherwise we can't have 2 orders pointing to the same vault, which is needed to buy/sell in a loop

Comment thread cli/src/cli/addorder/addorder.rs Outdated
constants : constants
} ;

let rain_magic_number = String::from("ff0a89c674ee7874") ;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use the magic numbers from the meta repo enum

Comment thread cli/src/cli/addorder/addorder.rs Outdated
let rain_magic_number = String::from("ff0a89c674ee7874") ;

// TODO cbor encode order_meta
let meta_string = hex::decode(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use cbor encoding from metadata repo

Comment thread cli/src/cli/deposit/mod.rs Outdated
).await ;

// Reinit Wallet Instance
let wallet= Ledger::new(HDPath::LedgerLive(0), chain_id).await?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you run cargo fmt over this codebase?

Comment thread cli/src/cli/withdraw/mod.rs Outdated
@@ -0,0 +1,139 @@

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

documentation

Comment thread cli/src/cli/removeorder/removeorder.rs Outdated
@@ -0,0 +1,46 @@
use std::{convert::TryFrom, sync::Arc};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

documentation

Comment thread cli/src/cli/removeorder/mod.rs Outdated
@@ -0,0 +1,100 @@
use clap::Parser;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

documentation

Comment thread cli/src/cli/mod.rs
@@ -1,29 +1,66 @@
use anyhow::Result;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

documentation

Comment thread cli/src/cli/listorders/mod.rs Outdated
@@ -0,0 +1,17 @@

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

documentation

@Siddharth2207 Siddharth2207 mentioned this pull request Sep 14, 2023
Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

this doesn't seem finished to me, lots of logic in ts that should sit in rust

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants