Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix TxViewer when no to (contract deployment) #4847

Merged
merged 5 commits into from
Mar 10, 2017
Merged

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Mar 9, 2017

Fixes #4799

@ngotchac ngotchac added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M8-dapp 💎 Decentralized applications. labels Mar 9, 2017
@@ -256,23 +259,29 @@ export class LocalTransaction extends BaseTransaction {

const newTransaction = {
from: transaction.from,
to: transaction.to,
Copy link
Collaborator

Choose a reason for hiding this comment

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

That doesn't seem to be right. What if the transaction actually has valid to?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, just noticed that we're adding it later on. I don't understand how does it fix the problem though. Previously transaction.to was set to 0x so we will set it anyway, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's added after :) (L283)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the issue was that the to field was set, and thus formatted to hex: '' => '0x'.
I guess the API should be modified in order to take this empty to case into account.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeap. Current fix does the trick, i.e. not encoding it.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Mar 9, 2017
@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 10, 2017
@gavofyork gavofyork merged commit be87151 into master Mar 10, 2017
@gavofyork gavofyork deleted the ng-fixlocaltx branch March 10, 2017 09:08
jacogr pushed a commit that referenced this pull request Mar 10, 2017
* Added React Hot Reload to dapps + TokenDeplpoy fix

* Fixes to the LocalTx dapp

* Don't send the nonce for mined transactions

* Don't encode empty to values for options
@jacogr jacogr mentioned this pull request Mar 10, 2017
arkpar pushed a commit that referenced this pull request Mar 10, 2017
* Added React Hot Reload to dapps + TokenDeplpoy fix (#4846)

* Fix method decoding (#4845)

* Fix contract deployment method decoding in Signer

* Linting

* Fix TxViewer when no `to` (contract deployment) (#4847)

* Added React Hot Reload to dapps + TokenDeplpoy fix

* Fixes to the LocalTx dapp

* Don't send the nonce for mined transactions

* Don't encode empty to values for options

* Pull steps from actual available steps (#4848)

* Wait for the value to have changed in the input (#4844)

* Backport Regsirty changes from #4589

* Test fixes for #4589
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M8-dapp 💎 Decentralized applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants