Closed
Conversation
## Motivation There are several `#[allow(dead_code)]` annotations without an explaining comment or link to an issue. Some of them seem to be unnecessary. ## Proposal Remove them. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation The `linera-core` unit tests don't run a whole network, and instead wrap a worker in a mutex inside a clonable `LocalValidatorClient` struct. To simulate faults, a `FaultType` enum value is stored alongside the worker inside the mutex. Since `ChainClient` sometimes cancels tasks that wait for a response from the validator, and we don't want to cancel the actual worker code in random places, we `spawn` tasks with the calls to the worker. (Maybe with the chain state actors that isn't necessary anymore? But that's a bigger discussion, and such a change could cause new flakiness if we're missing something.) These spawned tasks have a handle on the mutex with the worker state and the fault type. That means the spawned tasks can outlive calls to `ChainClient` functions in the tests. A `set_fault_type` call after such a function has returned can still affect how the spawned tasks behave. I strongly suspect this is what causes the very rare `test_memory_skipping_proposal` failure in CI, where we observe that a validator that should have had fault type `DontProcessValidated` ends up processing a validated block certificate anyway, because _after_ the client call that had spawned that task, we call `set_fault_type(… Honest)`. ## Proposal Put the fault type in the `LocalValidatorClient`, so it is copied before the task is spawned. ## Test Plan CI I will close linera-io#4422; let's reopen it if it happens again anyway. ## Release Plan - These changes should be backported to the testnet and devnet. ## Links - Fixes linera-io#4422. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation `AccountOwner::Address20`'s `Debug` impl prints something like: ``` Address20([10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10]) ``` because the `hex_debug` attribute is in the wrong place. ## Proposal Put the attribute directly on the field, not the enum variant. The `Debug` impl now says: ``` Address20(0a0a0a0a0a0a0a0a..) ``` ## Test Plan Tried it locally. ## Release Plan - Should be backported to testnet and devnet. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
…a-io#4457) ## Motivation In linera-io#4437 I used the existing `currentValidators` faucet query in `wallet init` and then made the client synchronize the initial admin chain from a committee where all these validators have the same weight. ## Proposal Add a `currentCommittee` query that returns the actual current committee, and use that to synchronize. ## Test Plan The reconfiguration test exercises this. ## Release Plan - These changes should be backported to devnet and testnet, published in an SDK and deployed in the faucet. ## Links - Closes linera-io#4434. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
…inera-io#4413) ## Motivation The transaction tracker is a field in the runtime but also passed into some execution state methods. In some cases, we take the tracker from the runtime, send it to the execution state actor, send it back and put it back into the runtime field. This is confusing and error-prone. ## Proposal Make the `ExecutionStateActor` a struct that borrows the state, tracker and resource controller. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation We simulate a range of validator faults in the `linera-core` tests. One of them is called `Malicious`, but it really just initializes the storage with wrong chains and returns `ArithmeticError`s in some cases. After linera-io#3787 (comment), the wrong initialization even causes the chains to have different chain ID, not just a wrong balance, so they will often return `InactiveChain` errors. Even before that chain, the storage initialization fault didn't play well with `set_fault_type`: Even if we switch them to `Honest`, they will still be faulty due to their storage contents. ## Proposal Replace `Malicious` with `NoChains`: Storage _does_ get initialized, but these validators return `InactiveChain` anyway, when handling block proposals, certificates or chain info queries. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (But might as well be backported: It's test only and not problematic.) ## Links - Closes linera-io#3858. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation We try to avoid `let _ =` in general: linera-io#4413 (comment) ## Proposal Deduplicate the code to send timing information in the chain client implementation, and log a warning if sending fails. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (Would be no problem to backport.) ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation This test is `#[ignore]`d because it's flaky: If the faulty validators respond before the other ones start processing the proposal, neither of them will have the proposal in their chain manager. Also, `test_sparse_sender_chain` is flaky: If the randomly chosen validator (the first in the committee, i.e. by public key) is faulty, processing the notification will fail. ## Proposal Explicitly handle the proposal in one of the validators to make sure the test is set up for the intended scenario. In `test_sparse_sender_chain`, use two validators with no faulty one, so the selected validator is guaranteed to have the block. ## Test Plan I ran it 20 times locally and it always passed. ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (Backporting would be no problem.) ## Links - Closes linera-io#3860. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation Some `ExecutionRequest` variants are unused. (My bad.) ## Proposal Remove them. We should actually make them private, but that's not straightforward. I'll try that in a separate PR. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
…4471) ## Motivation In the execution code, oracles must always be used as follows: * In replay mode, we consume an oracle response from the recorded ones. * If _not_ in replay mode, we query the actual oracle to obtain the value. * In both cases, we have to add it to the oracle responses in the execution outcome. It's currently easy to forget one of those steps. ## Proposal Refactor the transaction tracker's methods to make it a bit harder to use them the wrong way. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (But could easily be backported.) ## Links - Closes linera-io#3519. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation `NewRound` notifications are mainly needed for single-leader mode, where they are triggered by timeout certificates. However, in multi-leader mode, in case of failed proposals, they are also useful, so that the other owners can avoid making a conflicting proposal in the same round. ## Proposal Send `NewRound` notifications in `handle_block_proposal`, too. ## Test Plan An assertion was added to a worker test. ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - Closes linera-io#4472. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation In `test_utils` we `#![allow(unused_imports)]` because "items are only used by some tests" and "Rust will complain". That doesn't seem to be true, and if it were I don't see why the affected test shouldn't just import the items in question directly. ## Proposal Remove the attribute and the unused imports. Also, remove or restrict some similar attributes in other places. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
deuszx
approved these changes
Sep 4, 2025
Contributor
deuszx
left a comment
deuszx
left a comment
There was a problem hiding this comment.
Just a note that with this PR, CLI clients won't work with non-upgraded testnet.
Contributor
|
Thanks. Let's not squash backport commits, though |
Contributor
Author
|
There's only the "Squash and merge" button. Should I just |
ma2bd
requested changes
Sep 5, 2025
Contributor
|
pushed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Several backward-compatible fixes have been merged to the main branch after
testnet_conway.Proposal
Backport them.
Test Plan
CI
There were no cherry-picking conflicts.
Release Plan
Links
dead_codeattributes. #4454fault_typepart ofLocalValidatorClient. #4455Address20debug format. #4453currentCommitteeto the faucet; use it inwallet init. #4457MaliciouswithNoChains. #4462send_timingmethod; log errors. #4463test_finalize_validated. #4464NewRoundnotifications after proposals. #4473