Skip to content

Conversation

@jamescowens
Copy link
Member

This removes the bOPReturnEnabled flag and corrects
the handling of OP_RETURN outputs.

Note TX_NULL_DATA behavior is corrected.

Unfortunately a vout loop is required in TransactionRecord::showTransaction(const CWalletTx &wtx) to suppress the transactions that are not IsFromMe() and have a vout that is OP_RETURN. Inelegant, but effective.

This removes the bOPReturnEnabled flag and corrects
the handling of OP_RETURN outputs.
{
for (auto const& txout : wtx.vout)
{
if (txout.scriptPubKey == (CScript() << OP_RETURN))
Copy link
Member

Choose a reason for hiding this comment

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

the logic behind this is that if any tx with multiple txs contains this with no address in scriptpubkey it will display in all wallets. This allows OP_RETURN to be used with data in a transaction still however if its only a OP_RETURN and no valid address we just don't show the transaction.

The gibberish address we experienced and saw is the checksum of null for an address. Other coins experienced similar in the past.

Copy link
Member

@iFoggz iFoggz left a comment

Choose a reason for hiding this comment

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

tACK 484d5f7

tested and worked correctly in gui and rpc. balance also shows correctly.

@iFoggz
Copy link
Member

iFoggz commented Sep 2, 2018

#1281 #1282

@jamescowens
Copy link
Member Author

Also rebased to dev and tested "burn 5". Transaction was successful and shows correctly, as it is supposed to if it originates from you.

@denravonska denravonska merged commit e196b5e into gridcoin-community:hotfix Sep 3, 2018
denravonska added a commit that referenced this pull request Sep 3, 2018
Fixed
 - Fix burned coins incorrectly showing up in wallets, #1283 (@jamescowens).
 - Fix decimal output in RPC commands, #1272 (@Foggyx420).
 - Fix verbose flag in `getrawtransaction` RPC output, #1271 (@jamescowens).
@tomasbrod
Copy link
Member

You saved the day @jamescowens .

@jamescowens
Copy link
Member Author

Not really. An inelegant solution as a temporary measure... The removal of the flag and changes to IsMine and ExtractDestination are spot on. The filter loop on transaction display is inelegant, and as you mentioned, should probably be removed once we develop a repair command to remove the UTXO's out of everyone's wallets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants