Implement darkside getaddresstxids support#551
Conversation
LarryRuane
left a comment
There was a problem hiding this comment.
The GetTaddressTxids method has been deprecated, but it was actually recently renamed to GetTaddressTransactions (see around line 255 of service.proto). This rename was done because the old name was confusing, since this method returned full transactions, not just txids. (And it looks like this misled you too when writing this PR!)
So this PR will need to implement the functionality of GetTaddressTransactions, which will affect the two darkside methods you added; i.e., the darksideAddressTxid type will have to store transactions instead of txids.
ee0a38c to
c58b262
Compare
LarryRuane
left a comment
There was a problem hiding this comment.
Looks good; one small variable name suggestion. Maybe you can include this patch also (I think to the existing commit), may as well use this new feature to test GetTaddressTransactions (you can try this out by running ./smoke-test.bash).
Actually, when testing with those smoke test changes, I did run into one strange problem that took me a while to figure out. But I don't think any changes are needed in this PR to address it. At the end of the smoke test, I used the new AddAddressTransaction test method to add a transaction at a certain height, and was surprised when the subsequent GetTaddressTransactions returned the transaction as expected, but at a different height.
The problem turned out to be that the smoke test had earlier added this same transaction by way of adding a block that contained it (to either the active or pending block list), and the darkside getrawtransaction stub was finding it there, instead of in the new list you added. (And the transaction isn't expected to exist in more than one place, so it returns the first one it finds.)
To get around this problem, I added a call to the Reset test method to wipe out all previous state (i.e., those blocks and thus this transaction) before calling these new methods. Not the most elegant solution, but I don't think it's worth spending a lot of time to figure out a better way. I guess it is possible that someone writing wallet test code could run into this problem. One solution might be to have the darkside getrawtransaction stub check your new list first (instead of last).
One other problem I ran into is that I didn't notice that the struct you added, darksideAddressTransaction, has the same name as the protobuf-generated struct, DarksideAddressTransaction, except for the capitalization of the first letter. I was getting these two structs confused.
And the struct you added stores the transaction bytes in a field named txBytes, whereas the protobuf one uses Data. Maybe you can not add the new struct, and just use the protobuf version? It is a little larger (since it has some additional protobuf fields), but since this is only test code, that's probably okay. Or, if you keep the separate struct, maybe name it differently so it's less likely people will confuse them (as I did).
suggested testing patch diff
--- a/smoke-test.bash
+++ b/smoke-test.bash
@@ -414,10 +414,11 @@ expected='{
}'
compare "$expected" "$actual"
+tx='BAAAgIUgL4kAAAAAAADQHgoAECcAAAAAAAABBrP5A6pbmS1LPEu2DcnvwwbdbPr55uz++PwBiNeKaNuKYKCKg77MJXxKq/M55QOfNt86+QErujLjGjd6KHx8H8a2Swrvim7+j16PHKjPn9uR6eeS9nJveWzOvCcFJwx4/ajg6rjm7YRZu/MI8tUq/qlYHr5OlmhYIOonioW6xr25PQxLHBjeh2le4aqWhyD19ZdNl/tOmIJZSdTP3/F+EMv//DgBptxlKQm5o9X0tnS2B+BaRoZ3gDtz7eysGXfnMmjLGHtoB0wN57watCE6yTdJRdpc1NSyqVurSYr4qaMQ15FixeUq/R2H/uG6/sB24yt3+GRVa+cB/y/XnE/6X1DLXvzXT+YT55lrheCsqJ6oO4GuzPqPZqSIjLhDkhqnKJEbm+fEZf9IczlU1KpvKIqQo+lA+/kQdOZVk3vJdeXCtQDxzq7QyHqmwBjTtSeDYpg2w3iBh3yZPn2WofMaDVQfbU9xlDDykCc96GYdzBgtW4Eb41E1Ise1h40mKZYFAmXnBblph5HeX+wJbl2aN1+aScWr2WDa2dWQ1i95oDECpe/G9q13FyE6vAhrTPzIGpU5Dht5DvJTuc9zmTEx0gWrDkw+xGh7xo6e9n9UHevUlr9aQdvnZNVTW/IeMYQ6p0drjdwh2oI+zqPujvVXgaxOseHUNOyZdfK1TNu7xoZdSKO+ZRWAhwNUq1e9mbvm4MDIpUwVTZ5GhBWmqlkdk4opotQxvZBBynCuazC/zM/sjZ26V0I3nFmiMa0br71obCd9nGRUi4hVJjhF0OFnLSpta9pZ0J22SNvl1tetow22vIYeGqMLgNMP2t9S+wYx/Wnu4q//QY7kUhFe3HWY7QaKa4VQ9pyFoJPrAs6+KxA11M6IyzmEYB0ZXKh9hxnqE5nhiC8CoaKW4VP7nWrV4ACZhe4qEsY8K+HPj4/yugm3WdRKZa6CMMS2Y01EpMGajDeXCwGw7CJoU/H1acqr/Jf/iryp73PXf+QH4j06R6UlL+J3kwonYUnMoYG4GlISUnne3WRweXrK4RlIm2EyoskVePSTkrLBfGbV38qEBuVZc3clxogTMwiM3hjqlSPwgBJ+ypAIYBMJbk2fDwvkPp7nulS5xb0hkJexLmJaDBTmdN6+rUyRypXDTfZFTWPXUhq6Q5s0iugirXr5H9Z3RZQCVjgKuFMNBsXctd7Yv7amAsI3Q5B1R6UFODD12Qtgbw+W1TJxihlkhkyjDg+HMKtduCmxrPcW7Raw4HsOeLYcCIrNDP2b2n8rOsguIl5SkgCcYt5mAsAbZI5Xn7Jekgc8EUmua7tHFsX8+nI/87ZvyuUphbtcz8QFUuYnoMugiJPUCjSpzYwYWWvZXpEHNTTTvTfZrPbFRL7WG0dhgHGHxf4SeFo5dLd8zINIfEEbkXNELU1OTpkPu6RpZPmTiGg6hGOkeQdivLgSOO57BphexDd63aUVEz8EW4TIp3g+Vj3jY3TgUBP10cIrP0XK+n3+1qWB/Ged2cBYxm7tZnpgxqn4u7OKW+rVn2fdX5bvUqEd1na1EfCH9NoH0+cha98DKO9ehcYIN0REJYGG2uhZJ1xP2pImvPZjfK7xEGA3/LJfdtkx4BrdyDaN0e3N6CebMialcpzZDQaLpoghzUO0ouUO6wYnCYeitD4N0neK/K69RzgsEBQjV9+pP0L/dC+MPk0M+K8N4iOStVDnY6XTVti7vY2VC2UnsjNka64akO7qHo5Zru0EkecZmZ5aX1Y0eT6zoGcMReSIwENcfDHy7ccSRn9B1kFYuKU0fYqu6s1tryUVVOHdvLFZzLarCzQQGmUE3rQ9DMmk7RaWGf9q0uOYQ7FieHL/TE8Z+QCcS/IJfkBTU0I6VPMgXWqWmsCO4ZRSPn3Uak7lWzlZOyYJbfk+KtjGwR4z25A0/xlbF4Ax4YoRUIs8b4sxyDVylW1yp79yIdT+HNX9wcvkyx9oNcgBt5UgEbkeuHO1AdBE31Qp8L+BNAupTMirXt0nPedLwWLLu7zihTIkGB7abX+mpP1zkwpOzkeQHIs0ihSEI37Yq127LaxHkL9ZE2U80mgoIBsf+MZlRxIHTWTI2k4A+JiN2s/+uFc46u273345L1ZtmToU5OeGpTz3MfHKkfrS45vNev+2NfbTVEbztsF37ORHuGLw+DKgrLhuZke4VUXOQUtQnbfHjnWOGGVGyIkbbOlx/DutGw5vQjQKruP2M7vfDGhnAALpW+1ODgd53mvznREbkQbxmrI5YQZVnAWg8oxNM5fC2uCos/MsQiDUiwPsSzEKtoL9nEOJMIl32uF/rQKPiQjHVhLbmaRL+0vVPrRydRP8aJF23FEpOd7us7E/qPPfruaN8AQilZdU77dDjvklDsxfn0625rr6Iu24P99oSWcitAWF/FB7IKkuskzbKEJ83EPNozgN4/3xn2reJ1pEPuOGWYr30qyPhu6qQKfDqkCeCP3ZgeJcYTp/NbYsJ8aijSxKGvNF+1DUR8JFqFIMSUJrM+OuOZF/AEBYvUNQPWeAEoGf0A2l+mp4NiVBt8i3QejxpKkfOsqcR/01cp2Gf4wsLMIQ5/Kzf5ah63UTNo31m70bnZHKLKLIM50OTzZagShrQ6ChxtcXf+IYbtZOx/ryb8y3rxTAg5JSVheX7JvCRm9DnY4wXE9Jbsh+uyDQGTA+yo1WxieBYjxolGcc5nKwGCrs9KEPnChAofQqloRJlEWJshMUXVHuvxl8AV/+xl+36QPOop+n8ME2fbExY0B26Z5pL3nMZvVL9fgX5KFfmaP4d75QTmhui3tzgD33gTwi6f+uQU5foSYFoIO8ek97nl7N1jk3afBiCpyLl6oaE6PxyPc9/Ch8OSqKtmvSW4fp2UtMIVnle5Grrr/EjBWjrksDWRMe7qca4HklD8cb6RNB+n2Ed3aU0SuuyqNOaD0uVZh7twW1RB+cZyVh3z6M1mnncw6Ho8Kw+bDG5DCaK4O0h2TqrFOr2UvpZKdXSDG9mOSpvCjC8XcOmV0HhMi/YY4G5nPndlUzZySlEgzb6AD5v3UUBkBs+VAmggvzw+eKeqcXaywQh/LtEDsQJT30Wi+BT7plFFTRq+PqquWRic1At6TbgO2D417Nkp8nvr0M'
echo GetTransaction ...
actual=$(gp GetTransaction '{"hash": "H0nPz83r1cuQhdn/LvvNqHEh3aE/LHkRE/zy55uoIQg="}')
expected='{
- "data": "BAAAgIUgL4kAAAAAAADQHgoAECcAAAAAAAABBrP5A6pbmS1LPEu2DcnvwwbdbPr55uz++PwBiNeKaNuKYKCKg77MJXxKq/M55QOfNt86+QErujLjGjd6KHx8H8a2Swrvim7+j16PHKjPn9uR6eeS9nJveWzOvCcFJwx4/ajg6rjm7YRZu/MI8tUq/qlYHr5OlmhYIOonioW6xr25PQxLHBjeh2le4aqWhyD19ZdNl/tOmIJZSdTP3/F+EMv//DgBptxlKQm5o9X0tnS2B+BaRoZ3gDtz7eysGXfnMmjLGHtoB0wN57watCE6yTdJRdpc1NSyqVurSYr4qaMQ15FixeUq/R2H/uG6/sB24yt3+GRVa+cB/y/XnE/6X1DLXvzXT+YT55lrheCsqJ6oO4GuzPqPZqSIjLhDkhqnKJEbm+fEZf9IczlU1KpvKIqQo+lA+/kQdOZVk3vJdeXCtQDxzq7QyHqmwBjTtSeDYpg2w3iBh3yZPn2WofMaDVQfbU9xlDDykCc96GYdzBgtW4Eb41E1Ise1h40mKZYFAmXnBblph5HeX+wJbl2aN1+aScWr2WDa2dWQ1i95oDECpe/G9q13FyE6vAhrTPzIGpU5Dht5DvJTuc9zmTEx0gWrDkw+xGh7xo6e9n9UHevUlr9aQdvnZNVTW/IeMYQ6p0drjdwh2oI+zqPujvVXgaxOseHUNOyZdfK1TNu7xoZdSKO+ZRWAhwNUq1e9mbvm4MDIpUwVTZ5GhBWmqlkdk4opotQxvZBBynCuazC/zM/sjZ26V0I3nFmiMa0br71obCd9nGRUi4hVJjhF0OFnLSpta9pZ0J22SNvl1tetow22vIYeGqMLgNMP2t9S+wYx/Wnu4q//QY7kUhFe3HWY7QaKa4VQ9pyFoJPrAs6+KxA11M6IyzmEYB0ZXKh9hxnqE5nhiC8CoaKW4VP7nWrV4ACZhe4qEsY8K+HPj4/yugm3WdRKZa6CMMS2Y01EpMGajDeXCwGw7CJoU/H1acqr/Jf/iryp73PXf+QH4j06R6UlL+J3kwonYUnMoYG4GlISUnne3WRweXrK4RlIm2EyoskVePSTkrLBfGbV38qEBuVZc3clxogTMwiM3hjqlSPwgBJ+ypAIYBMJbk2fDwvkPp7nulS5xb0hkJexLmJaDBTmdN6+rUyRypXDTfZFTWPXUhq6Q5s0iugirXr5H9Z3RZQCVjgKuFMNBsXctd7Yv7amAsI3Q5B1R6UFODD12Qtgbw+W1TJxihlkhkyjDg+HMKtduCmxrPcW7Raw4HsOeLYcCIrNDP2b2n8rOsguIl5SkgCcYt5mAsAbZI5Xn7Jekgc8EUmua7tHFsX8+nI/87ZvyuUphbtcz8QFUuYnoMugiJPUCjSpzYwYWWvZXpEHNTTTvTfZrPbFRL7WG0dhgHGHxf4SeFo5dLd8zINIfEEbkXNELU1OTpkPu6RpZPmTiGg6hGOkeQdivLgSOO57BphexDd63aUVEz8EW4TIp3g+Vj3jY3TgUBP10cIrP0XK+n3+1qWB/Ged2cBYxm7tZnpgxqn4u7OKW+rVn2fdX5bvUqEd1na1EfCH9NoH0+cha98DKO9ehcYIN0REJYGG2uhZJ1xP2pImvPZjfK7xEGA3/LJfdtkx4BrdyDaN0e3N6CebMialcpzZDQaLpoghzUO0ouUO6wYnCYeitD4N0neK/K69RzgsEBQjV9+pP0L/dC+MPk0M+K8N4iOStVDnY6XTVti7vY2VC2UnsjNka64akO7qHo5Zru0EkecZmZ5aX1Y0eT6zoGcMReSIwENcfDHy7ccSRn9B1kFYuKU0fYqu6s1tryUVVOHdvLFZzLarCzQQGmUE3rQ9DMmk7RaWGf9q0uOYQ7FieHL/TE8Z+QCcS/IJfkBTU0I6VPMgXWqWmsCO4ZRSPn3Uak7lWzlZOyYJbfk+KtjGwR4z25A0/xlbF4Ax4YoRUIs8b4sxyDVylW1yp79yIdT+HNX9wcvkyx9oNcgBt5UgEbkeuHO1AdBE31Qp8L+BNAupTMirXt0nPedLwWLLu7zihTIkGB7abX+mpP1zkwpOzkeQHIs0ihSEI37Yq127LaxHkL9ZE2U80mgoIBsf+MZlRxIHTWTI2k4A+JiN2s/+uFc46u273345L1ZtmToU5OeGpTz3MfHKkfrS45vNev+2NfbTVEbztsF37ORHuGLw+DKgrLhuZke4VUXOQUtQnbfHjnWOGGVGyIkbbOlx/DutGw5vQjQKruP2M7vfDGhnAALpW+1ODgd53mvznREbkQbxmrI5YQZVnAWg8oxNM5fC2uCos/MsQiDUiwPsSzEKtoL9nEOJMIl32uF/rQKPiQjHVhLbmaRL+0vVPrRydRP8aJF23FEpOd7us7E/qPPfruaN8AQilZdU77dDjvklDsxfn0625rr6Iu24P99oSWcitAWF/FB7IKkuskzbKEJ83EPNozgN4/3xn2reJ1pEPuOGWYr30qyPhu6qQKfDqkCeCP3ZgeJcYTp/NbYsJ8aijSxKGvNF+1DUR8JFqFIMSUJrM+OuOZF/AEBYvUNQPWeAEoGf0A2l+mp4NiVBt8i3QejxpKkfOsqcR/01cp2Gf4wsLMIQ5/Kzf5ah63UTNo31m70bnZHKLKLIM50OTzZagShrQ6ChxtcXf+IYbtZOx/ryb8y3rxTAg5JSVheX7JvCRm9DnY4wXE9Jbsh+uyDQGTA+yo1WxieBYjxolGcc5nKwGCrs9KEPnChAofQqloRJlEWJshMUXVHuvxl8AV/+xl+36QPOop+n8ME2fbExY0B26Z5pL3nMZvVL9fgX5KFfmaP4d75QTmhui3tzgD33gTwi6f+uQU5foSYFoIO8ek97nl7N1jk3afBiCpyLl6oaE6PxyPc9/Ch8OSqKtmvSW4fp2UtMIVnle5Grrr/EjBWjrksDWRMe7qca4HklD8cb6RNB+n2Ed3aU0SuuyqNOaD0uVZh7twW1RB+cZyVh3z6M1mnncw6Ho8Kw+bDG5DCaK4O0h2TqrFOr2UvpZKdXSDG9mOSpvCjC8XcOmV0HhMi/YY4G5nPndlUzZySlEgzb6AD5v3UUBkBs+VAmggvzw+eKeqcXaywQh/LtEDsQJT30Wi+BT7plFFTRq+PqquWRic1At6TbgO2D417Nkp8nvr0M",
+ "data": "'$tx'",
"height": "663195"
}'
compare "$expected" "$actual"
@@ -428,3 +429,33 @@ expected='{
"errorMessage": "0821a89be7f2fc1311792c3fa1dd2171a8cdfb2effd98590cbd5ebcdcfcf491f"
}'
compare "$expected" "$actual"
+
+echo -n Reset to test GetTaddressTransactions ...
+gt Reset '{"saplingActivation": 663150,"branchID": "bad", "chainName":"x"}'
+
+echo -n GetTaddressTransactions ...
+gt AddAddressTransaction '{"address": "t1Yc45TyqLoNMBP1Ae4JM413ckTyvP2TUGA", "data": "'$tx'","height": 644337}'
+actual=$(gp GetTaddressTransactions '{"range":{"start":{"height":644337},"end":{"height":650510}},"address":"t1Yc45TyqLoNMBP1Ae4JM413ckTyvP2TUGA"}')
+expected='{
+ "data": "'$tx'",
+ "height": "644337"
+}'
+compare "$expected" "$actual"
+
+echo GetTaddressTransactions at range end ...
+actual=$(gp GetTaddressTransactions '{"range":{"start":{"height":2},"end":{"height":644337}},"address":"t1Yc45TyqLoNMBP1Ae4JM413ckTyvP2TUGA"}')
+expected='{
+ "data": "'$tx'",
+ "height": "644337"
+}'
+compare "$expected" "$actual"
+
+echo GetTaddressTransactions out of range start ...
+actual=$(gp GetTaddressTransactions '{"range":{"start":{"height":644338},"end":{"height":650510}},"address":"t1Yc45TyqLoNMBP1Ae4JM413ckTyvP2TUGA"}')
+expected=''
+compare "$expected" "$actual"
+
+echo GetTaddressTransactions out of range end ...
+actual=$(gp GetTaddressTransactions '{"range":{"start":{"height":2},"end":{"height":644336}},"address":"t1Yc45TyqLoNMBP1Ae4JM413ckTyvP2TUGA"}')
+expected=''
+compare "$expected" "$actual"
common/darkside.go
Outdated
| if err != nil { | ||
| return nil, errors.New("failed to parse getaddresstxids JSON") | ||
| } | ||
| txids := make([]string, 0) |
There was a problem hiding this comment.
I think this variable should no longer be called txids.
There was a problem hiding this comment.
Done -- renamed to matchingTxids.
Add staging for address-transaction associations in darkside mode so wallet clients using GetTaddressTransactions can detect transparent outputs during integration testing. Follows the existing AddAddressUtxo/ClearAddressUtxo pattern. Store full transaction bytes rather than just txids, making the helper self-contained: callers don't need to separately stage the transaction into a block for getrawtransaction to find it. The getaddresstxids handler parses stored transactions to compute txids on the fly, and darksideGetRawTransaction searches address transactions directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename txids variable to matchingTxids for clarity - Use protobuf DarksideAddressTransaction struct directly instead of custom darksideAddressTransaction to avoid naming confusion - Add GetTaddressTransactions smoke test coverage with range boundary tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c58b262 to
10b7f61
Compare
|
Thanks for the thorough review Larry. Pushed updates addressing all feedback:
|
Summary
Implement the
getaddresstxidsRPC in darkside test mode so wallet clients (e.g. Zingo) that useGetTaddressTransactionscan detect transparent outputs during integration testing. Previously this returned"not implemented yet".Fixes #455
Approach
Adds a staging mechanism for address-transaction associations following the existing
AddAddressUtxo/ClearAddressUtxopattern:DarksideAddressTransactionwith address, raw transaction bytes (data), and height fieldsAddAddressTransactionandClearAddressTransactionsonDarksideStreamergetaddresstxidshandler in darkside mode parses each stored transaction to compute its txid, then filters by address and block rangedarksideGetRawTransactionalso searches stored address transactions, sogetrawtransactioncan find them without requiring separate staging into blocksGetTaddressTransactionsflow works end-to-end:getaddresstxidsreturns txids, thengetrawtransactionreturns the full transaction bytesStoring full transaction bytes (rather than just txids) makes the helper self-contained -- callers don't need to separately stage the transaction into a block for
getrawtransactionto find it.Test plan
go build ./...compiles cleanlygo vet ./common/... ./frontend/...passes with no new warningsgo test ./...all existing tests passAddAddressTransactionwith address/raw-tx/height, then callGetTaddressTransactionsand verify the transaction is returnedGenerated with Claude Code