Conversation
…ransferHistory (#1823) This commit introduces a new struct called UriMeta that covers the necessary info about a token: image, token_name, description, attributes, and animation_url. It also adds new fields to NftTransferHistory such as collection_name and status. The collection_name field shows the name of the collection that the token belongs to. The status field indicates whether the transfer status is Receive or Send.
…ent versions (#1837) This commit updates the log, getrandom and wasm-bindgen dependencies to more recent versions that are inline with the latest libp2p upstream. Signed-off-by: ozkanonur <work@onurozkan.dev>
Adds a CI step that validates the pull request title to ensure that it complies with the conventional commit specifications. A pipeline will be triggered upon opening a pull request and upon renaming the pull request title. Signed-off-by: ozkanonur <work@onurozkan.dev>
…ght (#1841) KMD interest calculation is adjusted to reduce AUR (Active User Rewards) from 5% to 0.01% starting from KMD block height `3484958` (Fri Jun 30 2023) according to [KIP-0001](https://github.com/KomodoPlatform/kips/blob/main/kip-0001.mediawiki) fixes #1840
|
Needs this #1845 to be ready |
mm2src/coins/utxo.rs
Outdated
| // Some of these lines are ported as is from Komodo codebase | ||
| minutes -= 59; | ||
| let accrued = (value / 10_512_000) * minutes; | ||
| let mut accrued = (value as f64 * AUR_PER_MINUTE) as u64 * minutes; |
There was a problem hiding this comment.
I'm just curious why we need to use floating point arithmetic here, convert from integer to float and back, use drop_mutability! macros, rather than using something simpler like:
let accrued = if height >= N_S7_HARDFORK_HEIGHT {
(value / 10_512_000) * minutes / 500
} else {
(value / 10_512_000) * minutes
};There was a problem hiding this comment.
Thanks for your feedback. The code you provided is better than using drop_mutability!, I will fix it in a final fixes PR that I will open.
As for using floating point arithmetic, it was to make the code more understandable as suggested here #1841 (comment) , I think I will also go with your suggestion to avoid multiple type castings, I can write a comment above the code to make it more understandable. What do you think @caglaryucekaya since the previous change was your suggestion?
There was a problem hiding this comment.
I agree that conversion from integer to float is not necessary, but I think using constants is better than having magic numbers around the code. So I suggest the following:
// MINUTES_PER_YEAR = 365 * 24 * 60
const MINUTES_PER_YEAR: u64 = 525_600;
// Minutes required for 100% active user reward before N_S7_HARDFORK_HEIGHT
const MINUTES_PER_AUR: u64 = 20 * MINUTES_PER_YEAR;
let accrued = if height >= N_S7_HARDFORK_HEIGHT {
(value / MINUTES_PER_AUR) * minutes / 500
} else {
(value / MINUTES_PER_AUR) * minutes
};* remove nft and history len checks * transfer_list.len() > 0 check * use is_empty for list
| checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" | ||
| dependencies = [ | ||
| "getrandom 0.2.6", | ||
| "getrandom 0.2.9", |
There was a problem hiding this comment.
secure code reviewed rust-random/getrandom@v0.2.6...v0.2.9
| "proc-macro2 1.0.58", | ||
| "quote 1.0.27", |
There was a problem hiding this comment.
@DeckerSU plz cross-validate (secure code review)
| trie-root = "0.16.0" | ||
| uuid = { version = "1.2.2", features = ["fast-rng", "serde", "v4"] } | ||
| wasm-timer = "0.2.4" | ||
| instant = { version = "0.1.12" } |
There was a problem hiding this comment.
minor feedback: this one doesn't look maintained and relies on stdweb which isn't maintained neither. i.e we might want to consider using an alternative or fork it over and useweb-sys instead of stdweb for better future compatibility as its actively maintained and generally recommended for new projects targeting WASM support.
There was a problem hiding this comment.
wasm-timer is deprecated and we can remove it from our dependency tree (it's only used in our libp2p fork, which will be removed once we switch to upstream).
But removing instant is not very possible to do. Lots of core dependencies already depending on it. We basically removed wasm-timer from dependency tree and used instant without adding anything to dependency tree.
| serde_json = { version = "1", features = ["preserve_order", "raw_value"] } | ||
| serde_derive = "1.0" | ||
| wasm-bindgen = { version = "0.2.50", features = ["nightly"] } | ||
| wasm-bindgen = "0.2.86" |
There was a problem hiding this comment.
wasm-bindgen/wasm-bindgen@0.2.50...0.2.86 is quite massive and will require additional time. cc @DeckerSU @Alrighttt will need help - also might make sense to split it up between us and continue the complete (other 2/3) review AFTER co-approval.
| "futures-timer", | ||
| "kv-log-macro", | ||
| "log 0.4.14", | ||
| "log 0.4.17", |
There was a problem hiding this comment.
https://diff.rs/getrandom/0.2.0/0.2.9/
https://diff.rs/log/0.4.8/0.4.17/
https://diff.rs/quote/1.0.18/1.0.27/
https://diff.rs/proc-macro2/1.0.39/1.0.58
https://diff.rs/wasm-bindgen-shared/0.2.78/0.2.86/
instant 0.1.12
Reviewed the above and all changes.
There are two additional updated dependencies with very large diffs, https://diff.rs/syn/1.0.95/2.0.16/ and https://diff.rs/wasm-bindgen/0.2.50/0.2.86/ .
I am currently making my way through wasm-bindgen.
ca333
left a comment
There was a problem hiding this comment.
sec reviewed adex contributions @ 3b9aa92e00c66ddddc859905cf92b4984d346f9b
sec reviewed deps via local clone and ran comparison/integrity check against .cargo/registry:
rust-random/getrandom@v0.2.6...v0.2.9
dtolnay/proc-macro2@1.0.37...1.0.58
dtolnay/quote@1.0.18...1.0.27
https://github.com/sebcrozet/instant/tree/v0.1.12
rust-lang/log@0.4.14...0.4.17
dtolnay/syn@1.0.95...2.0.16
I believe Will update the release date in the changelog then merge this PR. |
Features:
Enhancements/Fixes:
wasm-timerdependency was removed from mm2 tree #1836log,getrandomandwasm-bindgendependencies were updated to more recent versions that are inline with the latest libp2p upstream #1837nS7HardforkHeightto comply with KIP-0001 #1841