trufflesuite / drizzle-legacy

Reactive Ethereum datastore for dapp UIs.
http://truffleframework.com/docs/drizzle/getting-started
MIT License
502 stars 129 forks source link

if reject transaction in metamask undefined key is added in transaction object #134

Closed nahuely closed 5 years ago

nahuely commented 6 years ago

if you click on reject transaction in metamask a undefined key is added in the transaction object, its not a critical bug i think, but at first glance its weird, im running the latest version of metamask extension and the latest version of drizzle 1.2.4, cheers

image

pd: i think is because there is not txhash , because the transaction hasnt even started

joshma91 commented 6 years ago

@nahuely you're right that there's no txHash because the transaction is never broadcasted. What do you think of the proposed solution below where in the absence of a txHash, we use the stackId as the key. This way there's a history of which transactions are rejected.

selection_013

nahuely commented 5 years ago

@joshma91 i think this is a posible solution, but i feel that we are loosing consistency in doing this, i mean, we are using the index of the transactionStack as a property in the transaction object(where there is no value), and when there is a value in the transactionStack when are using it as a key in the transaction, so i dont know if what im saying makes sense? cheers

joshma91 commented 5 years ago

@nahuely yea I totally get it. The transactionStack value is normally the key in transactions, and then in my case, the transactionStack key becomes the key in transactions which is inconsistent. I can't really think of a clean solution to this without re-engineering it so that a txHash is broadcast before Metamask approval.

maxme commented 5 years ago

Not being able to know if the transaction has been rejected is a big problem. I had to hack my way to change the state of my views when a transaction has been rejected, and it's really dirty.

As a first version of the fix, having the stackId in the transactions object would be much cleaner than having undefined. I agree it's inconsistent, but that's a first step.

Note: this is related to https://github.com/trufflesuite/drizzle/issues/86

honestbonsai commented 5 years ago

How about we always write a value (e.g. temp_SOME_UNIQUE_VALUE) to transactionStack array before any tx has been broadcasted?

Once broadcasted, overwrite that temp value in the transactionStack. If there's an error that results in no tx being broadcasted (e.g. rejecting a tx), use the temp value as the key in transactions object and we can still look it up using that unique temp value.

Keeps the consistency of the values in transactionStack mapping to the keys in the transaction obj.