Fix various ledger-tool error due to no builtins by ryoqun · Pull Request #12759 · solana-labs/solana

ryoqun · 2020-10-09T15:04:34Z

Problem

at least there is 3 bugs because of this:

Only the solana-validator code-path correctly populates builtins including bpf loader. That's because new_banks_from_ledger populates them using solana_core::builtins. The same treatment is needed for various ledger-tool commands.

Summary of Changes

Move builtins from core to ledger (we can't go runtime because of circular dep.). And ensure both solana-validator and solana-ledger-tool populates builtins via the common codepath of solana-ledger crate.

Context

Introduced in #12490

ryoqun · 2020-10-09T15:06:13Z

    pub new_hard_forks: Option<Vec<Slot>>,
    pub frozen_accounts: Vec<Pubkey>,
    pub debug_keys: Option<Arc<HashSet<Pubkey>>>,
-    pub additional_builtins: Option<Builtins>,


Removed additional_builtins from the ProcessOptions. Because this isn't rather optional. Now, the solana-ledger crate is responsible for populating this.

ryoqun · 2020-10-09T15:48:15Z

FYI: @carllin This is the fix of the libsolana_bpf_loader.so issue.

codecov · 2020-10-09T16:19:31Z

Codecov Report

Merging #12759 into master will decrease coverage by 0.0%.
The diff coverage is 50.0%.

@@            Coverage Diff            @@
##           master   #12759     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         353      353             
  Lines       84340    84338      -2     
=========================================
- Hits        69101    69093      -8     
- Misses      15239    15245      +6

t-nelson · 2020-10-09T16:25:11Z

https://buildkite.com/solana-labs/rolling-upgrade-tests/builds/124#978736eb-97d8-491a-9315-336de20c9189 👀

ryoqun · 2020-10-09T16:43:29Z

@t-nelson it progressed but stumbled on different error:

[2020-10-09T16:43:02.534504634Z INFO  solana_core::validator] local broadcast address: 0.0.0.0:8008
[2020-10-09T16:43:02.534508727Z INFO  solana_core::validator] local repair address: 0.0.0.0:8006
[2020-10-09T16:43:02.534513776Z INFO  solana_core::validator] local retransmit address: 0.0.0.0:8005
[2020-10-09T16:43:02.537066978Z INFO  solana_core::validator] Starting PoH: epoch=0 slot=0 tick_height=64 blockhash=2xn2dVXociUKgwBRYpgtFy1JA4JKiYytPgnsqvyePojf leader=Some(GCE2ajBbbxKATt1eMo37U2MvPcd
DtfoSbGSffe7zsiy1)
[2020-10-09T16:43:02.537129741Z INFO  solana_core::poh_recorder] poh_record: max_tick_height 64 reached, clearing working_bank 0
[2020-10-09T16:43:02.539819148Z INFO  solana_core::rpc_service] rpc bound to V4(0.0.0.0:8899)
[2020-10-09T16:43:02.539846235Z INFO  solana_core::rpc_service] rpc configuration: JsonRpcConfig { enable_validator_exit: true, enable_set_log_filter: true, enable_rpc_transaction_history: false, iden
tity_pubkey: boot1Z6jb15CLqpaMTn2CxktktwZpRAVAgHZEW6SxQ7, faucet_addr: Some(V4(127.0.0.1:9900)), health_check_slot_distance: 150, enable_bigtable_ledger_storage: false, enable_bigtable_ledger_upload: 
false }
[2020-10-09T16:43:02.546235056Z INFO  solana_core::rpc_pubsub_service] rpc_pubsub bound to V4(0.0.0.0:8900)
[2020-10-09T16:43:02.546269911Z INFO  solana_core::rpc_pubsub_service] rpc_pubsub max_payload: 15728640
[2020-10-09T16:43:02.546428214Z INFO  solana_net_utils::ip_echo_server] bound to Ok(V4(0.0.0.0:8001))
[2020-10-09T16:43:02.551024737Z ERROR solana_core::validator] Ledger does not have enough data to wait for supermajority, please enable snapshot fetch. Has 0 needs 1
./multinode-demo/bootstrap-validator.sh: line 140: kill: (22100) - No such process
############## validator exited, restarting ##############
./multinode-demo/bootstrap-validator.sh: line 116: kill: (22100) - No such process
solana-validator --gossip-host 35.230.102.118 --gossip-port 8001 --init-complete-file init-complete-node.log --wait-for-supermajority 1 --expected-bank-hash FtkRs76Ahk8vZfc7WRyzjx3GP9464B77qyWj2CVJxkz
1 --enable-rpc-exit --enable-rpc-set-log-filter --require-tower --ledger /home/solana/solana/net/../config/bootstrap-validator --rpc-port 8899 --snapshot-interval-slots 200 --identity /home/solana/sol
ana/net/../config/bootstrap-validator/identity.json --vote-account /home/solana/solana/net/../config/bootstrap-validator/vote-account.json --rpc-faucet-address 127.0.0.1:9900 --log -
pid: 22561
[2020-10-09T16:43:03.435533314Z INFO  solana_validator] solana-validator 1.3.16 (src:devbuild; feat:3843417642)

t-nelson · 2020-10-09T16:46:44Z

D'oh! This one's on me, in the scripts

t-nelson · 2020-10-09T17:07:23Z

This one is looking better! https://buildkite.com/solana-labs/rolling-upgrade-tests/builds/125#4312544a-2bef-44ff-9e44-e172bea7f914

t-nelson · 2020-10-09T17:30:52Z

Ok, this gets v1.3 off the ground. Great work, thanks!

The upgrade to v1.4 is failing now:

For some reason multinode-demo/validator.sh is trying to recreate accounts on restart. This should be fairly easy to resolve
We're also specifying --require-tower, but v1.3 isn't persisting it (right?). The node started up after removing the flag, but the cluster had halted by that time. I'm not sure what the correct fix is here

ryoqun · 2020-10-09T17:34:20Z

Ok, this gets v1.3 off the ground. Great work, thanks!

yay!

2\. We're also specifying `--require-tower`, but v1.3 isn't persisting it (right?).

This is true.

2\. the cluster had halted by that time.  I'm not sure what the _correct_ fix is here

how halted? Also, v1.3 cluster is halted? Is there some logs?

mvines · 2020-10-09T17:37:05Z

We're also specifying --require-tower, but v1.3 isn't persisting it (right?). The node started up after removing the flag, but the cluster had halted by that time. I'm not sure what the correct fix is here

I think we probably drop --require-tower in v1.4, and make it required in v1.5. v1.4 still uses the saved tower if present, but we're not quite ready to require it

t-nelson · 2020-10-09T17:41:42Z

how halted?

It halted expectedly. We lost supermajority due to 40% of stake going off line because of the above issues

Run logs https://buildkite.com/solana-labs/rolling-upgrade-tests/builds/125#4312544a-2bef-44ff-9e44-e172bea7f914. .14 is the node I revived, .176 the bootstrap validator, others dead due to loss of supermajority

t-nelson · 2020-10-09T17:43:29Z

I think we probably drop --require-tower in v1.4, and make it required in v1.5. v1.4 still uses the saved tower if present, but we're not quite ready to require it

Ok, great! This is what I was hoping, just wasn't sure if the default behavior was "load if present"

t-nelson · 2020-10-09T17:44:37Z

@ryoqun does merging this to master with v1.4 tag and merging my manual v1.3 bp sound good to you?

ryoqun · 2020-10-09T17:47:32Z

@ryoqun does merging this to master with v1.4 tag and merging my manual v1.3 bp sound good to you?

Yeah, that sounds good. :)

ryoqun · 2020-10-09T17:52:20Z

@t-nelson Please feel free to merge this & backport as you please if you want this quickly. I'm going go to bed. I've confirmed the remaining two of the three bugs in this pr's description are fixed on separate prs: #12690 (CI is green) and #12175 (this failed by unrelated reason, though; re-runnung). So, overall I'm pretty confident with this pr, now. :)

t-nelson · 2020-10-09T18:11:20Z

@Mergifyio refresh

mergify · 2020-10-09T18:11:52Z

Command refresh: success

Pull request refreshed

* Fix various ledger-tool error due to no builtins * Add missing file... (cherry picked from commit 1f4bcf7) # Conflicts: # core/Cargo.toml # ledger/Cargo.toml

* Fix various ledger-tool error due to no builtins (#12759) * Fix various ledger-tool error due to no builtins * Add missing file... (cherry picked from commit 1f4bcf7) # Conflicts: # core/Cargo.toml # ledger/Cargo.toml * Rebase Co-authored-by: Ryo Onodera <ryoqun@gmail.com> Co-authored-by: Michael Vines <mvines@gmail.com>

Fix various ledger-tool error due to no builtins

6093ecb

ryoqun commented Oct 9, 2020

View reviewed changes

Add missing file...

d341fe4

This was referenced Oct 9, 2020

Adjust bpf epoch for tests, not effecting prod. #12387

Merged

Introduce sanity/compatibility test for live clusters #12175

Closed

ryoqun requested a review from mvines October 9, 2020 15:25

ryoqun added v1.3 labels Oct 9, 2020

t-nelson mentioned this pull request Oct 9, 2020

Ryos compat fixes 1.3 #12762

Merged

ryoqun removed the v1.3 label Oct 9, 2020

t-nelson merged commit 1f4bcf7 into solana-labs:master Oct 9, 2020

mergify Bot mentioned this pull request Oct 9, 2020

Fix various ledger-tool error due to no builtins (bp #12759) #12766

Merged

ryoqun mentioned this pull request Oct 14, 2020

vote hash mismatch in testnet with high tps #12670

Closed

Conversation

ryoqun commented Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Context

Uh oh!

ryoqun Oct 9, 2020

Choose a reason for hiding this comment

Uh oh!

ryoqun commented Oct 9, 2020

Uh oh!

codecov Bot commented Oct 9, 2020

Codecov Report

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

ryoqun commented Oct 9, 2020

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

ryoqun commented Oct 9, 2020

Uh oh!

mvines commented Oct 9, 2020

Uh oh!

t-nelson commented Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

ryoqun commented Oct 9, 2020

Uh oh!

ryoqun commented Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-nelson commented Oct 9, 2020

Uh oh!

mergify Bot commented Oct 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ryoqun commented Oct 9, 2020 •

edited

Loading

t-nelson commented Oct 9, 2020 •

edited

Loading

ryoqun commented Oct 9, 2020 •

edited

Loading