Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

refactor: rework @ganache/ethereum-address to extend ethereumjs/util's Address #3777

Merged
merged 21 commits into from
Nov 9, 2022

Conversation

MicaiahReid
Copy link
Contributor

No description provided.

@@ -93,7 +93,7 @@ export default class BlockManager extends Manager<Block> {
const extraTxs: GanacheRawBlockTransactionMetaData[] = [];
json.transactions.forEach((tx, index) => {
const blockExtra = [
Quantity.toBuffer(tx.from),
Address.toBuffer(tx.from),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using Quantity here previously definitely was a bug 😅

@MicaiahReid MicaiahReid marked this pull request as ready for review November 1, 2022 15:47
Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

Looks so much better now! Loving it!

src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
@MicaiahReid MicaiahReid force-pushed the combine-ejs-ganache-address branch from 0e7e2f4 to e3cafc9 Compare November 8, 2022 17:03
@MicaiahReid MicaiahReid force-pushed the combine-ejs-ganache-address branch from e3cafc9 to 465f5f1 Compare November 8, 2022 17:07
Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

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

Other than my one comment - this is 🔥 . I love not having to hack around the ejs-Address type.

src/chains/ethereum/address/index.ts Show resolved Hide resolved
@MicaiahReid MicaiahReid merged commit bdf31b4 into tech-debt-dudes Nov 9, 2022
@MicaiahReid MicaiahReid deleted the combine-ejs-ganache-address branch November 9, 2022 21:41
MicaiahReid added a commit that referenced this pull request Nov 14, 2022
MicaiahReid added a commit that referenced this pull request Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants