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

Refactor Transactions throughout core #243

Merged
merged 44 commits into from
Dec 11, 2018
Merged

Refactor Transactions throughout core #243

merged 44 commits into from
Dec 11, 2018

Conversation

davidmurdoch
Copy link
Member

Fixes issue where transaction hashes weren't correctly calculated.

nicholasjpaterno and others added 27 commits November 26, 2018 22:09
@nicholasjpaterno
Add ethers.js regression test, fix tx signing
7ad133f
@nicholasjpaterno
Refactor statemanager combine related conditionals
3f0e4bd
@nicholasjpaterno
Add missing await in forking.js
de4b9b3
@nicholasjpaterno
WIP attempting to sign 'eth_sendTransaction' when priv key is known
363894e
@nicholasjpaterno
Update forking.js small refactor
bbf5d1c
@davidmurdoch
WIP: normalize Transasction
0c2cb54
@davidmurdoch
WIP: more Transaction normalization
45826ee
@davidmurdoch
Fix some nonce stuff
6a060cf
@davidmurdoch
Set asyncRequestProcessing back to true
8d036a7
@davidmurdoch
more Transaction updates
a739932
@davidmurdoch
additional refactorings
5139298
@davidmurdoch
txhash as hex string is more consistent with other code
be30348
@davidmurdoch
just sign everything, for now
09a669e
@davidmurdoch
I dno't think data should ever be modified, even to include a 0x?
3c077c6
@davidmurdoch
eth_calls are always fakes?
0821847
@davidmurdoch
make sure we toLowerCase the from address
956591e
@davidmurdoch
Implement our own to.buffer method.
4248b24
@davidmurdoch
Sometimes address can be a string so we need to convert to a buffer
59ee582
@davidmurdoch
Make sure we reject non-string to in transactions
7cc6b90
@davidmurdoch
Fix typo
4c3c02e
@davidmurdoch
Copy+paste typo fix
4a9f622
@davidmurdoch
data and input are aliases in txs
07dea43
@davidmurdoch
not sure if this is right
db4ea64
@davidmurdoch
add the missing chain id calc back in
07e7a9c
@davidmurdoch
Clean up
1c868a4
@davidmurdoch
Update tests to use our Transaction instead of ethereumjs-tx
cd795ee
@davidmurdoch
more clean up
86de280
@davidmurdoch davidmurdoch mentioned this pull request Dec 4, 2018
Closed
4 tasks
nicholasjpaterno
nicholasjpaterno reviewed Dec 4, 2018
View reviewed changes
Copy link
Contributor

@nicholasjpaterno nicholasjpaterno left a comment

Choose a reason for hiding this comment

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

This all looks good to me, my only real question is with the static get types() values.
transaction.js line ~141

lib/utils/transaction.js Show resolved Hide resolved
lib/utils/transaction.js Outdated Show resolved Hide resolved
lib/utils/transaction.js Outdated Show resolved Hide resolved
lib/blockchain_double.js Show resolved Hide resolved
nicholasjpaterno
nicholasjpaterno approved these changes Dec 4, 2018
View reviewed changes
Copy link
Contributor

@nicholasjpaterno nicholasjpaterno left a comment

Choose a reason for hiding this comment

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

Looks good to me, so long as we never need to check (signed || fake) && none.

davidmurdoch and others added 15 commits December 4, 2018 16:58
nicholasjpaterno
nicholasjpaterno approved these changes Dec 11, 2018
View reviewed changes
Copy link
Contributor

@nicholasjpaterno nicholasjpaterno left a comment

Choose a reason for hiding this comment

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

This all LGTM!

@davidmurdoch davidmurdoch merged commit b2b89b1 into develop Dec 11, 2018
@davidmurdoch davidmurdoch deleted the ethersjs-test-integration branch December 18, 2018 02:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nicholasjpaterno nicholasjpaterno nicholasjpaterno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@davidmurdoch @nicholasjpaterno