-
Notifications
You must be signed in to change notification settings - Fork 12
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
xCheddar impl #14
base: master
Are you sure you want to change the base?
xCheddar impl #14
Conversation
add xCheddar lockup token upgrade cheddar to 4.0.0-pre 7 upgate fn migrate()
There was a problem hiding this 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neardev/dev-account.env
Outdated
@@ -0,0 +1 @@ | |||
CONTRACT_NAME=dev-1654515440352-14631167054094 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do
There was a problem hiding this 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)
cheddar/Cargo.toml
Outdated
@@ -11,10 +11,10 @@ crate-type = ["cdylib", "rlib"] | |||
[dependencies] | |||
serde = { version = "*", features = ["derive"] } | |||
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
near-sdk = "4.0.0-pre.7" | |
near-sdk = "4.0.0" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
cheddar/Cargo.toml
Outdated
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
near-contract-standards = "4.0.0-pre.7" | |
near-contract-standards = "4.0.0" |
cheddar/Cargo.toml
Outdated
uint = { version = "0.9.0", default-features = false } | ||
|
||
[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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
near-sdk-sim = "4.0.0-pre.7" | |
near-sdk-sim = "4.0.0" |
cheddar/README.md
Outdated
``` | ||
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 |
There was a problem hiding this comment.
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()), |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 { |
xcheddar/src/xcheddar.rs
Outdated
self.internal_stake(&sender_id, amount); | ||
PromiseOrValue::Value(U128(0)) | ||
} else { | ||
// deposit reward |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert_xcheddar(&xcheddar_info0, to_yocto("100"), 0, 0); | |
assert_xcheddar(&xcheddar_info0, total_reward, 0, 0); |
@robert-zaremba can we finalize and merge, so front-end devs can start work? |
self.assert_owner(); | ||
self.owner_id = owner_id.as_ref().clone(); | ||
assert!( | ||
env::is_valid_account_id(owner_id.as_bytes()), |
There was a problem hiding this comment.
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()), | ||
) |
There was a problem hiding this comment.
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(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why removing?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
add xCheddar lockup token
upgrade cheddar to 4.0.0-pre 7
upgate fn migrate()