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.

Sign up for GitHub

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

Jump to bottom

storage.last_height should be an Option<BlockHeight> #454

Closed
james-chf opened this issue Sep 8, 2022 · 1 comment · Fixed by #1993
Closed

storage.last_height should be an Option<BlockHeight> #454

james-chf opened this issue Sep 8, 2022 · 1 comment · Fixed by #1993
Assignees
@tzemanovic @Fraccaman
Labels
has-a-pr The issue has been solved by a PR that has yet to be merged ledger refactor / code quality

Comments

@james-chf
Copy link
Contributor

james-chf commented Sep 8, 2022 •
edited
Loading

In our Storage struct, the type of the last_height field (representing the height of the most recently committed block) is just BlockHeight, but when a chain is first initialized, no block has yet been committed so it would be clearer if this was Option<BlockHeight> and initially None rather than BlockHeight(0) before the first block has been committed (or some other way of refactoring this to be clearer).
@james-chf james-chf mentioned this issue Sep 8, 2022
Merged
@tzemanovic
Copy link
Member

it's kinda puke-y, but because cometBFT uses block height 1 for the first block, we can use 0 for when it's not initialized. This is being done in #1993

@tzemanovic tzemanovic mentioned this issue Oct 23, 2023
Merged
2 tasks
@tzemanovic tzemanovic added the has-a-pr The issue has been solved by a PR that has yet to be merged label Oct 23, 2023
phy-chain pushed a commit to phy-chain/namada that referenced this issue Mar 1, 2024
@jurevans @emccorson
feat/453 - SDK/Ledger Signing Updates - Support v0.28.0 (anoma#454)
73bd15d 
* Remove parentId for Ledger

* Refactor sdkStore

sdf

* Pass stored signing_key to sign_tx function to fix signing issues

* Remove SDK store from background

* Clean up spending key storage

* Remove password from SDK calls

* Upgrade to official zondax ledger package

* Fix bad dependency

* Logic to handle xsk or private key

* Add spendingKey back as stringified object for compatiblity

* Update to 0.28.0

* Namada v0.28.0 T->T transfers (anoma#521)

* WIP: Namada v0.28.0 code with T->T transfers working

* fix: update to support unbond tx, clean up

---------

Co-authored-by: Justin R. Evans <jurevans@gmail.com>

---------

Co-authored-by: Eric Corson <13696952+emccorson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tzemanovic tzemanovic

@Fraccaman Fraccaman

Labels
has-a-pr The issue has been solved by a PR that has yet to be merged ledger refactor / code quality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix the start block height of the first epoch anoma/namada
3 participants
@tzemanovic @Fraccaman @james-chf