Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/dcr: Remove GetRawTransaction. #3012

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Conversation

JoeGruffins
Copy link
Member

GetRawTransaction will error if txindex is not specified on the node, and until now GetTransaction has been good enough for client.

Until 12a90a7 we were only using GetRawTransaction for mempool redemptions, which it will not error for https://github.com/decred/dcrd/blob/7735cbb4e11b61636e88ad07e9ad8c1a30dd4047/internal/rpcserver/rpcserver.go#L2899

closes #2947

@JoeGruffins
Copy link
Member Author

I will fix tests if these changes look like the right direction. They may take a little work.

@@ -3787,12 +3779,12 @@ func (dcr *ExchangeWallet) findRedemptionsInMempool(contractOutpoints []outPoint
}

for _, txHash := range mempoolTxs {
tx, err := dcr.wallet.GetRawTransaction(dcr.ctx, txHash)
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't getrawtransaction work for mempool transaction even if txindex is not enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for mempool transactions.

Should we leave it in with a note that it can only be used for mempool?

My reasoning is that if GetTransaction is good enough for rpc clients with an spv node, then it's good enough for full nodes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to note for testing: the dcr simnet test harness node alpha has --txindex; node beta does not have a tx index.

Copy link
Member Author

Choose a reason for hiding this comment

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

beta wallet is also an --spv wallet so I dont think we have a simnet wallet connecting to beta node

@buck54321
Copy link
Member

Made you a conflict @JoeGruffins. Don't miss the spelling fix for received.

@JoeGruffins
Copy link
Member Author

Ill work on fixing the tests.

@@ -4654,7 +4655,7 @@ func TestFindBond(t *testing.T) {
}, {
name: "bad msgtx",
coinID: bond.CoinID,
txRes: txFn(nil, bond.SignedTx[1:]),
Copy link
Member Author

Choose a reason for hiding this comment

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

I have not idea why I had to change this, maybe I should.

GetRawTransaction will error if txindex is not specified on the node,
and until now GetTransaction has been good enough for client.
@buck54321 buck54321 merged commit 40d23e0 into decred:master Oct 30, 2024
5 checks passed
buck54321 pushed a commit that referenced this pull request Oct 30, 2024
GetRawTransaction will error if txindex is not specified on the node,
and until now GetTransaction has been good enough for client.
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.

dcr: rpc wallet needs --txindex with v1.0.0
3 participants