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

idb(postgres): add protocol version check before writing to database. #1506

Merged
merged 2 commits into from
Mar 15, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented Mar 14, 2023

Summary

Make sure the block being written is for a known protocol version.

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #1506 (f2fd79e) into develop (922653f) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1506      +/-   ##
===========================================
- Coverage    70.33%   70.31%   -0.02%     
===========================================
  Files           70       70              
  Lines         9680     9685       +5     
===========================================
+ Hits          6808     6810       +2     
- Misses        2375     2377       +2     
- Partials       497      498       +1     
Impacted Files Coverage Δ
idb/postgres/postgres.go 66.66% <100.00%> (-0.07%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -175,6 +175,12 @@ func (db *IndexerDb) init(opts idb.IndexerDbOptions) (chan struct{}, error) {

// AddBlock is part of idb.IndexerDb.
func (db *IndexerDb) AddBlock(vb *itypes.ValidatedBlock) error {
protoVersion := protocol.ConsensusVersion(vb.Block.CurrentProtocol)
_, ok := config.Consensus[protoVersion]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this reminds me of EnableAssetCloseAmount. do we still need to support AssetCloseAmount for the older txns? maybe it's one of the things we can deprecate in the next release?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to touch the API right now. This check is mainly to ensure you're running with up to date software. i.e. imagine a new txn field is added.

@winder winder marked this pull request as ready for review March 15, 2023 14:34
@winder winder merged commit cf0074c into algorand:develop Mar 15, 2023
@winder winder deleted the will/stop-unknown-protocol branch March 15, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants