Metadata upgrade #579
Merged
Metadata upgrade #579
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
swansontec
force-pushed
the
william/metadata-upgrade
branch
2 times, most recently
from
c2e88af
to
70efe80
Compare
Also upgrade the fake currency plugin so the test can access its tokens in the new format.
- Replace `mergeDeeply` with a type-safe merge routine. - Do more work in the cleaner, to avoid the separate `DiskMetadata` type.
swansontec
force-pushed
the
william/metadata-upgrade
branch
from
d8f282b
to
e37ac0a
Compare
paullinator
requested changes
Jan 4, 2024
| name: over.name ?? under.name, | ||
| notes: over.notes ?? under.notes | ||
| const out: EdgeMetadata = { exchangeAmount: {} } | ||
| const { exchangeAmount = {} } = out |
There was a problem hiding this comment.
Adding a default fallback not necessary. just do
const { exchangeAmount } = out
There was a problem hiding this comment.
This doesn't type-check because exchangeAmount is a ?:. The fallback is useless at runtime, but TypeScript is angry without it.
CHANGELOG.md
Outdated
| @@ -10,6 +10,7 @@ | |||
| - added: Allow deleting metadata fields by passing `null` to `saveTxMetadata`. | |||
| - deprecated: `EdgeCurrencyEngineCallbacks.onBalanceChanged`. Use `onTokenBalanceChanged` instead. | |||
| - deprecated: `EdgeTransaction.currencyCode`. Use `tokenId` instead. | |||
| - removed: Turn rate plugins into a no-op. These plugins may still be passed to `addEdgeCorePlugins`, and `EdgeAccount.rateCache` object still exists, but all rates are locked at 0. | |||
There was a problem hiding this comment.
This is borderline a breaking change. Sure the API stays the same but the plugins just plain not working is arguably a breaking change. It's even worse because a user won't even get a TS error that the API is broken. I would revert this.
Also store metadata on-disk using tokenId instead of currencyCode, with fallback support for reading old data.
This makes it possible to get transactions and addresses using a tokenId instead of a currency code.
swansontec
force-pushed
the
william/metadata-upgrade
branch
from
b2ca26d
to
d4a25c7
Compare
paullinator
approved these changes
Jan 4, 2024
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.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
This branch back-ports all the new non-breaking features from Paul's core 2.0 branches, so we can release them as a final v1 branch. Then we can merge the breaking v2 changes on top.