-
Notifications
You must be signed in to change notification settings - Fork 89
Conversation
Great stuff! 😄 Do we eventually want to get https://github.com/ethereumjs/merkle-patricia-tree/issues/108 (drop level dependency) in before we release? This sounded pretty significant from what to read from the issue description and it would need a breaking change, so this would be a good occasion here. |
@holgerd77 ah yes for sure, that sounds nice to include. |
This "Remove dependency on ethereumjs-util" #10 is also a candidate where we might want to do something within the scope of this release. Eventually the work from @cgewecke in the works on the util library around the discussion from ethereumjs/ethereumjs-util#228 would also already help here when we would integrate an updated |
Sounds good. I already removed all of the It looks like these are all the We could remove edit: |
Same here, let's give this release (again 😜) a couple of days more and see how discussion around ethereumjs/ethereumjs-util#250 evolve. |
Hi @ryanio, people seem to stumble upon somewhat hidden breaking changes on the upcoming v4 release, see e.g. thread in #118. Can you before release go once again through the listed PRs have a deeper look into where breaking changes are introduced and more clearly label (with "[BREAKING]" or something, or eventually also some oral extra section) the changes which are breaking? Think this is necessary to avoid frustration along the upgrade process. |
Ah ok, or is there too much still in motion from #118 ? |
Sorry, wanted to post this before the last comment here, have mixed this up along another thread: @ryanio Can we do an explicit update to Is this otherwise ready for a review? |
@holgerd77 I'd like to do a final check with @msieczko and @sz-piotr if they have drawn conclusions on any broken or inconsistent behavior that we might want to include in this release (see #118 (comment)) , otherwise it should be good! |
Hi Ryan, please take ethereumjs/ethereumjs-util#253 into account respectively upgrade on a new release before releasing, thanks! Think in this case we should also do here one other explicit upgrade (and tests run) on an eventual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed changes, all seems good.
…-copy Include checkpoints by default in SecureTrie.copy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested a small change in the example in CHANGELOG.md. Perhaps this change could also be applied to the README.md
wrap example in function Co-authored-by: gabrocheleau <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the await in front of test()
* rename Trie.prove to Trie.createProof tidy up typedoc * regen docs * add pr to changelog * move export type up
Does this need a final review/approval? |
@holgerd77 yes, please! i'm ready when you are to coordinate a release on npm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, had a last overall look, this looks good, also based on the review by @gabrocheleau (thanks!)!
This release introduces a major API upgrade from callbacks to Promises.
Example using async/await syntax:
Breaking Changes
Trie methods
See the docs for the latest Promise-based method signatures.
Trie.prove renamed to Trie.createProof
To clarify the method's purpose
Trie.prove
has been renamed toTrie.createProof
.Trie.prove
has been deprecated but will remain as an alias forTrie.createProof
until removed.Trie raw methods
getRaw
,putRaw
anddelRaw
were deprecated inv3.0.0
and have been removed from this release. Instead, please usetrie.db.get
,trie.db.put
, andtrie.db.del
. If using aSecureTrie
orCheckpointTrie
, usetrie._mainDB
to override the checkpointing mechanism and interact directly with the db.SecureTrie.copy
SecureTrie.copy
now includes checkpoint metadata by default. To maintain original behavior of not copying checkpoint state, passfalse
to paramincludeCheckpoints
.Changed
Nibbles
type fornumber[]
(#115)Added
Fixed
Dev / Testing / CI
_formatNode
(#109)failingRefactorTests
tosecure.spec.ts
(#110)Compare view
v3.0.0 => v4.0.0
Linked issues
Closes #10
Closes #102