nodes issueshttps://git.duniter.org/groups/nodes/-/issues2024-03-21T11:25:19+01:00https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/220Smith-members: invert `issuer` and `receiver` in events2024-03-21T11:25:19+01:00Cédric MoreauSmith-members: invert `issuer` and `receiver` in eventsruntime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/219Distance : rajouter le résultat dans l'évènement2024-03-21T11:25:19+01:00Cédric MoreauDistance : rajouter le résultat dans l'évènementActuellement les évènements _`EvaluatedValid` et `EvaluatedInvalid`_ n'ont que l' `idty_index` comme métadonnée.Actuellement les évènements _`EvaluatedValid` et `EvaluatedInvalid`_ n'ont que l' `idty_index` comme métadonnée.runtime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/222Compilation error with try-runtime feature2024-03-21T11:24:52+01:00Benjamin GalloisCompilation error with try-runtime featureRuntime not compiling with the `try-runtime` feature:
- Implementation is not up-to-date.
- Downstream error occurs where `try-runtime` requires traits implemented only with `runtime-benchmarks`.
- Deprecation: `use of deprecated method ...Runtime not compiling with the `try-runtime` feature:
- Implementation is not up-to-date.
- Downstream error occurs where `try-runtime` requires traits implemented only with `runtime-benchmarks`.
- Deprecation: `use of deprecated method `try_runtime_cli::TryRuntimeCmd::run`: Substrate's `try-runtime` subcommand has been migrated to a standalone CLI (https://github.com/paritytech/try-runtime-cli). It is no longer being maintained here and will be removed entirely some time after January 2024. Please remove this subcommand from your runtime and use the standalone CLI.`Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/215Document compilation features2024-03-21T11:24:51+01:00Hugo TrentesauxDocument compilation featuresWe have multiple compilation features that should be documented for new contributors.We have multiple compilation features that should be documented for new contributors.Horizonhttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/216Reduce code duplication for runtime features2024-03-21T00:00:57+01:00Hugo TrentesauxReduce code duplication for runtime featuresA lot of code duplication happens in node definition like:
```rust
// with features {gdev, gtest, g1}
g1_runtime::RuntimeApi, g1_executor::G1Executor
```
Most of them could be refactored using imports, some would need basic macro, and ...A lot of code duplication happens in node definition like:
```rust
// with features {gdev, gtest, g1}
g1_runtime::RuntimeApi, g1_executor::G1Executor
```
Most of them could be refactored using imports, some would need basic macro, and this could make reading and editing this part of the code easier.Horizonhttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/203Handlers simplification2024-03-20T23:02:17+01:00Benjamin GalloisHandlers simplification## Issue
Currently, the problem lies in having complicated handlers to follow, namely `OnEvent`. This handler has multiple paths hidden in several implementations, making it harder to read and account for weight. The `OnIdty` is on the ...## Issue
Currently, the problem lies in having complicated handlers to follow, namely `OnEvent`. This handler has multiple paths hidden in several implementations, making it harder to read and account for weight. The `OnIdty` is on the same model but with fewer paths.
## Current Design
Your implementation currently presents two designs for handlers:
1. A loosely coupled approach where the handler is implemented in the runtime, and there is no tight coupling between dependencies.
2. A tightly coupled approach where the handler is implemented inside one or more pallets, and dependencies are tightly coupled. These handlers are then assembled in a tuple handler, or in a (super) handler in the runtime that call these handlers.
At the moment, both approaches are used simultaneously, with `OnEvent` being implemented loosely in the runtime but also tightly inside the `wot` pallet ([source](https://git.duniter.org/nodes/rust/duniter-v2s/-/blob/master/runtime/common/src/pallets_config.rs?ref_type=heads#L502)).
## Solution
There are multiple solutions to the problem:
1. Splitting the `OnEvent` into smaller handlers, making them more comprehensible, following the model of `OnNewcert` and `OnRemovecert`.
2. Implementing all handlers in a single implementation in the runtime, avoiding tuple handlers. This will make the pallets more flexible, avoiding tight couplings, but it can be counterintuitive for pallets like `duniter-wot`, where there is already a tight coupling with several pallets.
## Note
In !246, Hugo mentioned decoupling the logic and the manual weight accounting that needs to be done for functions called in handlers and hooks. One way to achieve this is to separate the pallet into internal private functions with `()` or `DispatchResult` return type and public external functions with a `Weight` return type. I created a small POC crate where the weight can be autogenerated based on an expression or a function of the same argument signature, see example below. So, it is entirely possible to go this way with minimal work if the simplification mentioned above is not enough.
```rust
#[derive_weight(Weight::from_parts(10, 10))]
fn do_something(i: u32, j: u32) -> u32 {
i * j
}
assert_eq!(weighted_do_something(1, 2), Weight::from_parts(10, 10));
//
fn weight_for_do_something(i: u32, _j: u32) -> Weight {
if i == 0 {
Weight::from_parts(10, 10)
}
else {
Weight::from_parts(20, 20)
}
}
#[derive_weight(weight_for_do_something)]
fn do_something(i: u32, j: u32) -> u32 {
if i == 0 {
i + j
}
else {
i/j
}
}
assert_eq!(weighted_do_something(0, 2), Weight::from_parts(10, 10));
assert_eq!(weighted_do_something(10, 2), Weight::from_parts(20, 20));
//
#[derive_weight((Weight::zero(), Weight::from_parts(10, 10)))]
fn do_something(i: u32, j: u32) -> Result<(), ()> {
if i == 0 {
Ok(())
} else {
Err(())
}
}
assert_eq!(weighted_do_something(0), weight::from_parts(0, 0));
assert_eq!(weighted_do_something(1), weight::from_parts(10, 10));
```Horizonhttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/163Split OnEvent(membership_event)2024-03-20T23:02:17+01:00Hugo TrentesauxSplit OnEvent(membership_event)Splitting `OnEvent(membership_event)` into
- OnMembershipAdded
- OnMembershipRenewed
- OnMembershipRemoved
would be easier to read. (dispatch instead of match)Splitting `OnEvent(membership_event)` into
- OnMembershipAdded
- OnMembershipRenewed
- OnMembershipRemoved
would be easier to read. (dispatch instead of match)runtime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/167Membership handler weight accounting2024-03-20T23:02:17+01:00Benjamin GalloisMembership handler weight accountingCurrently, an extrinsic and a hook in the pallet membership call `do_remove_membership`, which in turn calls the membership handler. This handler either calls `remove_member` in the authority-members pallet or `on_removed_member` in the ...Currently, an extrinsic and a hook in the pallet membership call `do_remove_membership`, which in turn calls the membership handler. This handler either calls `remove_member` in the authority-members pallet or `on_removed_member` in the universal-dividend pallet.
- `universal-dividend::on_removed_member` has a complexity parameter that is not taken into account in the actual accounting of every membership function calling `membership::do_remove_membership` in the wot instance.
- The `pallet::membership::revoke_membership` has a hidden complexity parameter, bonded by `T::MaxPastReeval` of the universal-dividend pallet, which poses a problem because `T::MaxPastReeval` is not accessible in the membership pallet, and secondly, because it is an instance pallet. One way to get past that is to use the `DispatchResultWithPostInfo` with weight accounting inside the extrinsic that will be instance-dependent and include the complexity parameter to avoid overcounting.
To do:
- [x] Propagate weight through the provider from `universal-dividend::on_removed_member` to `pallet_membership::do_remove_membership`.
- [x] Correct hook weight using the propagated weight.
- [x] Compute the extrinsic weight using `membership::do_remove_membership` with the worst-case scenario of `T::MaxPastReeval` i.e. arbitrary high block number during benchmark.
- [x] Use the `DispatchResultWithPostInfo` and the propagated weight to correct the weight used by the wot instance.runtime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/duniter-squid/-/issues/4Add foreign key on event in addition to blockNumber2024-03-20T12:07:30+01:00Hugo TrentesauxAdd foreign key on event in addition to blockNumberWhen I started the indexer, blockNumber was enough for my needs, since I then switched to RPC. But with the kind of usage in Cesium v2, we would need to get more info on the block details:
![image](/uploads/c63c4808f40eade34665767d622e1...When I started the indexer, blockNumber was enough for my needs, since I then switched to RPC. But with the kind of usage in Cesium v2, we would need to get more info on the block details:
![image](/uploads/c63c4808f40eade34665767d622e185f/image.png)![image](/uploads/57edcdbee07b9d3d393dfeb0ab4c40de/image.png)
> these clickable links goes to the block
See forum discussion: https://forum.duniter.org/t/duniter-squid/11858/12
---
So we could replace most blockNumber Int fields by a foreign key on the block.https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/214Allow native Runtime execution2024-03-08T11:36:25+01:00Cédric MoreauAllow native Runtime executionSince !229, the `--execution native` CLI option is not effective anymore. This was planned both by [Polkadot](https://www.polkadot.network/blog/a-polkadot-postmortem-24-05-2021) and [our team](https://forum.duniter.org/t/exemple-de-bug-l...Since !229, the `--execution native` CLI option is not effective anymore. This was planned both by [Polkadot](https://www.polkadot.network/blog/a-polkadot-postmortem-24-05-2021) and [our team](https://forum.duniter.org/t/exemple-de-bug-lie-a-la-difference-entre-runtime-wasm-et-natif/11574/10?u=cgeek).
However, I feel a bit lost when I cannot put Breakpoints in the Runtime while developing: I use theme a lot both for understanding Runtime behavior and global Client code call stack. It is very effective for my global comprehension of Substrate code.
I understand why we removed this option. I suggest that we bring it back in another form: a Cargo _feature_ _flag_ allowing for its usage by developers:
```plaintext
cargo run --feature native
```runtime-802Cédric MoreauCédric Moreauhttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/207Distance oracle tries to publish inherent even if already published result, l...2024-03-06T18:56:08+01:00Hugo TrentesauxDistance oracle tries to publish inherent even if already published result, leading to ExtrinsicFailed result of the inherentDistance oracle tries to publish inherent even if already published result, leading to ExtrinsicFailed result of the inherent
for more details about the example: https://forum.duniter.org/t/oracle-de-distance-dans-un-docker/11706/10Distance oracle tries to publish inherent even if already published result, leading to ExtrinsicFailed result of the inherent
for more details about the example: https://forum.duniter.org/t/oracle-de-distance-dans-un-docker/11706/10runtime-802https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/174Calibrate distance `MAX_EVALUATIONS_PER_SESSION`2024-03-06T18:56:07+01:00Hugo TrentesauxCalibrate distance `MAX_EVALUATIONS_PER_SESSION`After !219, `on_new_session` hook in distance pallet will perform non-trivial operation including `do_valid_distance_status` and `on_valid_distance_status` cascade with `try_add_membership`/`try_renew_membership` branches. This can be do...After !219, `on_new_session` hook in distance pallet will perform non-trivial operation including `do_valid_distance_status` and `on_valid_distance_status` cascade with `try_add_membership`/`try_renew_membership` branches. This can be done at most `MAX_EVALUATIONS_PER_SESSION` times, so this value should be set carefully to avoid spending more weights than a block has.
@bgallois pointed me that for session pallet, the whole weight of a block is consumed in `on_initialize`, no matter what is really consumed by `rotate_session`: https://github.com/paritytech/polkadot-sdk/blob/ac303406ffc0b1478b53d236a7983c31755717bc/substrate/frame/session/src/lib.rs#L566C5-L566C37
We would prefer to avoid that and only reserve a known worst-case weight that is known not to exceed the total available.runtime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/196Check that transfer_all on a linked account does not lead to empty linked acc...2024-03-06T15:23:57+01:00Hugo TrentesauxCheck that transfer_all on a linked account does not lead to empty linked accountNot sure which behavior we want to have but after #153 it would be better to simply unlink the account.Not sure which behavior we want to have but after #153 it would be better to simply unlink the account.runtime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/202align distance oracle on modulo instead of session2024-03-06T15:05:13+01:00Hugo Trentesauxalign distance oracle on modulo instead of sessionIn Duniter v1 the time for distance evaluation was 5 minutes. In Duniter v2 it's 2 hours. That's too much. And its also too strict because it is aligned on sessions which is cumbersome in development networks.
Quote from the forum:
htt...In Duniter v1 the time for distance evaluation was 5 minutes. In Duniter v2 it's 2 hours. That's too much. And its also too strict because it is aligned on sessions which is cumbersome in development networks.
Quote from the forum:
https://forum.duniter.org/t/distance/11672/16
[quote="HugoTrentesaux, post:16, topic:11672"]
[quote="tuxmain, post:6, topic:11672"]
On pourrait se synchroniser avec des numéros de blocs modulo N plutôt que des sessions, si besoin.
[/quote]
Ce serait pas mal de ramener ce temps d’évaluation à 5 minutes comme dans Duniter v1. Par contre il faudra trouver un remplaçant pour `on_new_session`. Ça pourrait être un `on_init` avec un modulo, tout simplement.
[/quote]
Linked to #174runtime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/duniter-squid/-/issues/12Define @cardinality for every type in graphql.schema2024-02-28T13:24:56+01:00pokaDefine @cardinality for every type in graphql.schemaThis is required to be able to define `--max-response-size` option to `squid-graphql-server` command, to prevent DoS.
**Source: https://docs.subsquid.io/sdk/resources/graphql-server/dos-protection/**This is required to be able to define `--max-response-size` option to `squid-graphql-server` command, to prevent DoS.
**Source: https://docs.subsquid.io/sdk/resources/graphql-server/dos-protection/**https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/191Misleading error message in logs for distance oracle2024-02-27T19:43:58+01:00Hugo TrentesauxMisleading error message in logs for distance oracleThe following log message can be seen when there is no (= zero) key in keystore but says that there is more than one.
```rust
let &[owner_key] = owner_keys else {
log::error!("🧙 [distance oracle] More than one Babe owner key: oracle...The following log message can be seen when there is no (= zero) key in keystore but says that there is more than one.
```rust
let &[owner_key] = owner_keys else {
log::error!("🧙 [distance oracle] More than one Babe owner key: oracle cannot work");
```runtime-802Carles BarrobésCarles Barrobéshttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/161Add live tests for membership status coherence2024-02-27T17:22:23+01:00Hugo TrentesauxAdd live tests for membership status coherenceMR !215 introduces new identity status which is a kind of index on other pallets. Live tests should be added to check storage consistency:
- unconfirmed → no pseudo is defined
- unvalidated → no membership exist
- member → a membership ...MR !215 introduces new identity status which is a kind of index on other pallets. Live tests should be added to check storage consistency:
- unconfirmed → no pseudo is defined
- unvalidated → no membership exist
- member → a membership exist
- notmember → no membership exist
- revoked → no membership existruntime-802Benjamin GalloisBenjamin Galloishttps://git.duniter.org/nodes/rust/duniter-v2s/-/issues/205use identity index as validator id2024-02-27T16:02:55+01:00Hugo Trentesauxuse identity index as validator id#197 is still #S-needdiscussion, but at least the "use identity index as validator id" can be done independently.#197 is still #S-needdiscussion, but at least the "use identity index as validator id" can be done independently.https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/180firstEligibleUd displays default value2024-02-24T14:11:15+01:00Hugo TrentesauxfirstEligibleUd displays default valueI see `firstEligibleUd: 1`
![image](/uploads/e06eb44e39f011247595a51dc6705318/image.png)
which is obviously not true since I did claim multiple UDs from the beginning and do not get new UDs when I claim multiple times.
Is it a polkado...I see `firstEligibleUd: 1`
![image](/uploads/e06eb44e39f011247595a51dc6705318/image.png)
which is obviously not true since I did claim multiple UDs from the beginning and do not get new UDs when I claim multiple times.
Is it a polkadotjs display bug or rpc problem? Or a problem with the way we modify this value?
Elois made heavy use of `core::mem::replace` (5 use in 3 pallets). I do not know if it's ok.https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/201distance oracle refuses "insecure url"2024-02-23T01:28:06+01:00Hugo Trentesauxdistance oracle refuses "insecure url"I tried to run indexer on a local docker network with ws:
```
docker compose logs -f
distance-oracle-distance-oracle-1 | thread 'main' panicked at /root/distance-oracle/src/api.rs:31:10:
distance-oracle-distance-oracle-1 | Cannot crea...I tried to run indexer on a local docker network with ws:
```
docker compose logs -f
distance-oracle-distance-oracle-1 | thread 'main' panicked at /root/distance-oracle/src/api.rs:31:10:
distance-oracle-distance-oracle-1 | Cannot create RPC client: Rpc(InsecureUrl("ws://duniter-smith:9944"))
distance-oracle-distance-oracle-1 | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
distance-oracle-distance-oracle-1 | Waiting 1800 seconds before next execution...
```
How to allow "insecure url"?runtime-802Hugo TrentesauxHugo Trentesaux