Skip to content
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

At Record Stage, it seems there is possibility about not hashing when tick event. #407

Closed
JhChoy opened this issue Jun 22, 2018 · 3 comments
Labels
question Add this to close an issue with instructions on how to repost as a question on Stack Exchange

Comments

@JhChoy
Copy link

JhChoy commented Jun 22, 2018

Please correct me if I'm wrong.
At record_stage.rs :: fn try_process_signals,
if let Some(entry) = recorder.tick(start_time, tick_duration) { sender.send(entry).or(Err(()))?; }

recorder.rs :: fn tick, it returns
Some(self.record(vec![]))

and at fn record
Entry::new_mut(&mut self.last_hash, &mut self.num_hashes, transactions)

goes to Entry:: fn new_mut,
let entry = Self::new(start_hash, *cur_hashes, transactions);
and reset num_hashes and start_hash
*start_hash = entry.id; *cur_hashes = 0;

At fn new, it calls next_hash and there,

if !hash_data.is_empty() {
        extend_and_hash(&id, &hash_data)
    } else if num_hashes != 0 {
        hash(&id)
    } else {
        id
    }

hash(&id) looks like to use for hashing tick event b/c tick event doesn't have data, just num_hashes.
However, when if tick event occurs just after recording entry with txs, tick event will have 0 num_hashes because it always resets after calling Entry::new_mut. Therefore, tick event couldn't be hashed.
I want you to let me know if I'm mistaken or if this is a bug.

@garious
Copy link
Contributor

garious commented Jun 25, 2018

Not exactly a bug, but a redundant entry. The first entry in the genesis block actually uses this "feature" to communicate the ledger's seed hash.

@rob-solana, did you coincidently fix exactly this last week? Maybe validators should probably reject ledgers with empty entries after the genesis' entry0? I can't think of how you'd use empty entries to perform an attack, but it does seem weird that empty entries would allow multiple valid ledgers to coexist.

@garious garious added the question Add this to close an issue with instructions on how to repost as a question on Stack Exchange label Jun 25, 2018
@mvines mvines added this to the The Future! milestone Sep 5, 2018
@garious
Copy link
Contributor

garious commented Jan 16, 2019

@rob-solana, ping

@rob-solana
Copy link
Contributor

I'm pretty sure there's no possibility of generating a hashless Entry in record any more. Entry test code can still generate weird entries, but PoH code is very strict.

@garious garious modified the milestones: The Future!, v0.11 Tabletops Jan 17, 2019
godmodegalactus pushed a commit to blockworks-foundation/solana that referenced this issue Jan 5, 2024
Use cluster info functions for tpu (solana-labs#345) (solana-labs#347)
Use git rev-parse for git sha
Remove blacklisted tx from message_hash_to_transaction (Backport solana-labs#374) (solana-labs#376)
Updates scripts for easy local setup. (solana-labs#383)
Backports sim_bundle improvements (solana-labs#407)
backports clone derivation 416 (solana-labs#417)
Backport solana-labs#419: add upsert to AccountOverrides (solana-labs#420)

backports solana-labs#430 v1.16: update jito-programs sha (solana-labs#431)

[JIT-1661] Faster Autosnapshot (solana-labs#406)

Fix Buildkite warnings (solana-labs#437)

Backport solana-labs#446 to v1.16 (solana-labs#447)

backport 428, runtime plugin (solana-labs#429)

v1.16: Backport solana-labs#449 (solana-labs#450)
lijunwangs pushed a commit to lijunwangs/solana that referenced this issue Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Add this to close an issue with instructions on how to repost as a question on Stack Exchange
Projects
None yet
Development

No branches or pull requests

4 participants