Conversation
| targets = ["x86_64-unknown-linux-gnu", "aarch64-apple-darwin", "wasm32-unknown-unknown"] | ||
|
|
||
| [dependencies] | ||
| codec = { workspace=true, default-features = false, features = ["derive"] } |
There was a problem hiding this comment.
I wonder why the formatting was still off here even after we did fmt...
There was a problem hiding this comment.
did we run taplo as well?
There was a problem hiding this comment.
Was kinda expecting fmt to also format toml files. But we could just taplo everything.
dastansam
left a comment
dastansam
left a comment
There was a problem hiding this comment.
some comments so far. but I would probably consider using OnUnbalanced trait instead of manually resolving imbalance.
Also, please, use debug or trace for logs, ideally, get rid of them. You already have an event that indicates the funds directed to treasury.
pallet shouldn't emit so much info logs (see substrate pallets for reference). if you want to debug in unit tests, add the logs and remove them once unit tests are passing.
| targets = ["x86_64-unknown-linux-gnu", "aarch64-apple-darwin", "wasm32-unknown-unknown"] | ||
|
|
||
| [dependencies] | ||
| codec = { workspace=true, default-features = false, features = ["derive"] } |
There was a problem hiding this comment.
did we run taplo as well?
| bytes[0] = id; | ||
| AccountId32::new(bytes) | ||
| } | ||
| pub struct TestCommons; |
There was a problem hiding this comment.
| pub struct TestCommons; | |
| pub struct TestEnv |
runtime/tests/common.rs
Outdated
|
|
||
| impl TestCommons { | ||
| // Add #[allow(dead_code)] attribute to suppress warnings | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
you won't have to add #[allow(dead_code)] if you create a tests/mod.rs file and mark the module in lib.rs as a test.
#[cfg(test)]
mod tests;There was a problem hiding this comment.
For integration tests, we don't have anything included in lib.rs, but I moved this attribute one level higher.
There was a problem hiding this comment.
hmm, why don't we include it in the runtime/lib.rs?
There was a problem hiding this comment.
It's one level higher, it's not one file in the same folder.
|
In general, it looks good to me. This might not be the right PR to change this, but we want a portion of the every block reward to go to the treasury, not just voluntary ones. Happy to discuss this more if you like, but the general idea is, let's have guaranteed funding for the treasury. |
illuzen
left a comment
illuzen
left a comment
There was a problem hiding this comment.
The only critical thing I think is check_payment seems seems off. Is the function unnecessary?
|
@illuzen |
|
@dastansam Our idea for rewards is quite simple. I already have OnUnbalanced working for transactions, but I don’t see a straightforward way to apply it to the treasury percentage. If you have a minute, could you show me how I can do this? |
runtime/tests/common.rs
Outdated
| // Call on_initialize for pallets that need it | ||
| System::on_initialize(b + 1); | ||
| resonance_runtime::Scheduler::on_initialize(b + 1); | ||
| pub fn print_balances() { |
There was a problem hiding this comment.
is this necessary? why not just print it directly?
runtime/tests/common.rs
Outdated
|
|
||
| impl TestCommons { | ||
| // Add #[allow(dead_code)] attribute to suppress warnings | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
hmm, why don't we include it in the runtime/lib.rs?
4 participants
Please check how this change.
For a better understanding of the concept, please check the documentation first.
https://quantus-network.github.io/docs/governance/