Skip to content

Conversation

@YellingOilbird
Copy link
Collaborator

add xCheddar lockup token
upgrade cheddar to 4.0.0-pre 7
upgate fn migrate()

add xCheddar lockup token
upgrade cheddar to 4.0.0-pre 7
upgate fn migrate()
Copy link
Contributor

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

partial review

README.md Outdated

## XCheddar token

Token which allows users stake their Cheddar tokens and get some rewards for it. Base model is the same as CRV and veCRV locked tokens model. After 30-days period distribution of rewards starts and it counting from monthly reward parameter. Locked token have a virtual price depends on amount of locked/minted XCheddar tokens
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add a links to CRV and veCRV?

Copy link
Contributor

Choose a reason for hiding this comment

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

ping

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@robert-zaremba yep, what’s required? I update all with ahead commits

Copy link
Contributor

Choose a reason for hiding this comment

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

links to explain CRV and veCRV

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -0,0 +1 @@
CONTRACT_NAME=dev-1654515440352-14631167054094 No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add neardev to .gitignore

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 do

Copy link
Contributor

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

  • let's don't include the compiled wasm code
  • the math for redeeming xCheddar is not clear (see the comments)
  • timestamp in seconds should be u64, not u32.
  • add docs about msg parameter for ft_transfer_call handler (ft_on_transfer)

serde_json = "*"
near-sdk = { git = "https://github.com/near/near-sdk-rs", tag="3.1.0" }
near-contract-standards = { git = "https://github.com/near/near-sdk-rs", tag="3.1.0" }
near-sdk = "4.0.0-pre.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
near-sdk = "4.0.0-pre.7"
near-sdk = "4.0.0"

Copy link
Contributor

Choose a reason for hiding this comment

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

v4 is already released, let's don't use -pre

Copy link
Contributor

Choose a reason for hiding this comment

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

we can already use v4.1.0-pre

near-contract-standards = { git = "https://github.com/near/near-sdk-rs", tag="3.1.0" }
near-sdk = "4.0.0-pre.7"
near-sys = "0.1.0"
near-contract-standards = "4.0.0-pre.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
near-contract-standards = "4.0.0-pre.7"
near-contract-standards = "4.0.0"

[dev-dependencies]
# near-primitives = { git = "https://github.com/nearprotocol/nearcore.git" }
# near-sdk-sim = { git = "https://github.com/near/near-sdk-rs.git", version="v3.1.0" }
near-sdk-sim = "4.0.0-pre.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
near-sdk-sim = "4.0.0-pre.7"
near-sdk-sim = "4.0.0"

```
rustup target add wasm32-unknown-unknown
RUSTFLAGS='-C link-arg=-s' cargo build --target wasm32-unknown-unknown --release
cp target/wasm32-unknown-unknown/release/cheddar_coin.wasm res/cheddar_coin.wasm
Copy link
Contributor

Choose a reason for hiding this comment

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

we have this in Makefile. Let's use make instruction here.

self.assert_owner();
self.owner_id = owner_id.as_ref().clone();
assert!(
env::is_valid_account_id(owner_id.as_bytes()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need to validate, AccountId should be validated automatically

Copy link
Contributor

Choose a reason for hiding this comment

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

AFIAK, v4 validates account automatically

}

// return the amount of to be distribute reward this time
pub(crate) fn try_distribute_reward(&self, cur_timestamp_in_sec: u32) -> Balance {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub(crate) fn try_distribute_reward(&self, cur_timestamp_in_sec: u32) -> Balance {
pub(crate) fn compute_distribute_reward(&self, cur_timestamp_in_sec: u32) -> Balance {

self.internal_stake(&sender_id, amount);
PromiseOrValue::Value(U128(0))
} else {
// deposit reward
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use a specific message for making reward desposits. Eg msg: "reward deposit" - otherwise users may run into troubles.
Also update the docs about that.

total_reward += to_yocto("100");

let xcheddar_info0 = view!(xcheddar_contract.contract_metadata()).unwrap_json::<ContractMetadata>();
assert_xcheddar(&xcheddar_info0, to_yocto("100"), 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert_xcheddar(&xcheddar_info0, to_yocto("100"), 0, 0);
assert_xcheddar(&xcheddar_info0, total_reward, 0, 0);

@htafolla
Copy link
Contributor

htafolla commented Jul 8, 2022

@robert-zaremba can we finalize and merge, so front-end devs can start work?

YellingOilbird added 2 commits July 21, 2022 01:43
self.assert_owner();
self.owner_id = owner_id.as_ref().clone();
assert!(
env::is_valid_account_id(owner_id.as_bytes()),
Copy link
Contributor

Choose a reason for hiding this comment

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

AFIAK, v4 validates account automatically

ext_ft_resolver::ext(env::current_account_id())
.with_static_gas(GAS_FOR_RESOLVE_TRANSFER)
.ft_resolve_transfer(sender_id, receiver_id, amount.into()),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to update to a new API, then let's remove the previous version. Although I would keep the original and only do minimal changes to already deployed token.

}
return used_amount.into();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why removing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I correctly remember I do this because of new standards version

@@ -0,0 +1,74 @@
# P3 (NFT versioned) explanation
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we call it p4-farm-nft? We already have p3 farm, and it's working totally differently.

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.

3 participants