Stop updating the wallet during linera benchmark #3367
Merged
+135
−96
Conversation
This file contains 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
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ndr-ds
force-pushed
the
02-18-stop_updating_the_wallet_during_linera_benchmark
branch
2 times, most recently
from
6ce896d to
d1250d4
Compare
ndr-ds
force-pushed
the
02-18-stop_updating_the_wallet_during_linera_benchmark
branch
from
d1250d4 to
94c5bf2
Compare
afck
reviewed
Feb 19, 2025
ndr-ds
force-pushed
the
02-18-stop_updating_the_wallet_during_linera_benchmark
branch
4 times, most recently
from
ebd651b to
10e8da9
Compare
afck
approved these changes
Feb 20, 2025
linera-client/src/client_context.rs
Outdated
Comment on lines
821
to
823
| chain_info.block_hash, | ||
| chain_info.timestamp, | ||
| chain_info.next_block_height, |
There was a problem hiding this comment.
This will always be
None,
timestamp_of_certificate_in_line_767,
BlockHeight::ZERO,won't it? I think the timestamp doesn't even matter as long as the local clock isn't reset to an earlier time, so we could set that to zero or now().
linera-service/src/linera/main.rs
Outdated
| // Below all block proposals are supposed to succeed without retries, we | ||
| // must make sure that all incoming payments have been accepted on-chain | ||
| // and that no validator is missing user certificates. | ||
| context.process_inboxes_and_force_validator_updates().await; | ||
| context | ||
| .process_inbox_without_updating_wallet(&chain_client) |
There was a problem hiding this comment.
But for the default chain we do want to update the wallet?
There was a problem hiding this comment.
True, I guess if something else changes it (if it's used for something other than the benchmark). Good point
There was a problem hiding this comment.
Also because of the blocks we created on the default chain to open the benchmark chains. The default chain has a greater block height after the benchmark than before.
linera-client/src/client_context.rs
Outdated
Comment on lines
670
to
677
| let info = self | ||
| .client | ||
| .local_node() | ||
| .clone() | ||
| .handle_certificate(certificate.clone(), &()) | ||
| .await? | ||
| .info; | ||
| chain_client.update_from_info(&info); |
There was a problem hiding this comment.
Alternatively, we could make process_certificate public?
Suggested change
| let info = self | |
| .client | |
| .local_node() | |
| .clone() | |
| .handle_certificate(certificate.clone(), &()) | |
| .await? | |
| .info; | |
| chain_client.update_from_info(&info); | |
| chain_client.process_certificate(certificate.clone()).await?; |
ndr-ds
force-pushed
the
02-18-stop_updating_the_wallet_during_linera_benchmark
branch
2 times, most recently
from
e4b2dac to
220ab76
Compare
afck
approved these changes
Feb 20, 2025
ndr-ds
force-pushed
the
02-18-stop_updating_the_wallet_during_linera_benchmark
branch
from
220ab76 to
56771fa
Compare
This was referenced Feb 20, 2025
ndr-ds
deleted the
02-18-stop_updating_the_wallet_during_linera_benchmark
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
For the chains we're using during the benchmark, we were currently adding them to our wallet. However, that is not necessary.
Proposal
Create chains for the benchmark, and don't add them to any wallet, just use them for the benchmark then close them.
Test Plan
Ran locally, saw that things worked correctly
Release Plan