-
Notifications
You must be signed in to change notification settings - Fork 47
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
integrate pallet teerex refactoring #1390
Conversation
* [local-setup] fix setup with two workers: avoid using the same data dirs. (#1358) * [ita_stf/test_genesis] remove reserved balance from state, as it doesn't exist anymore * [enclave-runtime/top_pool_execution] downgrade logs to trace * [state_handler] add debug and trace logs * add additional flags for the worker setup * [its-consensus-aura/block_importer] add some more logs * local setup add some more logging
I think the flaky M6 test might be mitigated by #1405 |
) -> Result<Vec<AuthorityId<P>>, ConsensusError> | ||
where | ||
ValidateerFetcher: ValidateerFetch + EnclaveOnChainOCallApi, | ||
P: Pair, | ||
P::Public: UncheckedFrom<[u8; 32]>, |
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.
Authorities as well are too generic IMO. We could use the same itp_types::AccountId type strictly
too much trait bound jungle here
@@ -881,16 +915,36 @@ fn enclave_account<E: EnclaveBase>(enclave_api: &E) -> AccountId32 { | |||
} | |||
|
|||
/// Checks if we are the first validateer to register on the parentchain. | |||
fn we_are_primary_validateer( | |||
fn we_are_primary_worker( |
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.
this logic has now changed to something more meaningful. Instead of testing if we are the first enclave to have ever registered on this chain ( which would allow only a single sidechain to ever exist) we now check shard_status and see if some other enclave than self has ever touched that shard
@clangenb there may be quite some additional cleanup left after this PR as a consequence, but I thought the PR is big enough already. |
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.
Cool, only some minor issues; otherwise this looks more straightforward than I expected it to be!
core-primitives/node-api/api-client-extensions/src/pallet_teerex.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: clangenb <37865735+clangenb@users.noreply.github.com>
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.
One minor thing, otherwise it looks good to me now! Will approve.
closes #1384
Merge this first:
done: