Skip to content
This repository has been archived by the owner on Jun 10, 2022. It is now read-only.

Embedded transactions cannot be filtered by address (instead of signerPublicKey) #433

Open
Vektrat opened this issue Jul 24, 2020 · 1 comment
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed P3 Issue Low severity cosmetic issues with minor inconvenience

Comments

@Vektrat
Copy link
Contributor

Vektrat commented Jul 24, 2020

Giving the address as alternative to signerPublicKey does not yield results for aggregate transactions even though it should (assuming meta.addresses has involved addresses in it, just like in regular transactions). However, aggregate transactions lack this meta field, leaving them with just signerPublicKey.

We considered asking core to add meta.addresses to aggregate transactions, however, since this is a specific case, and since signerPublicKey is a core transaction field (unlike for example, targetAddress which is specific to certain types of transaction), we can bypass this issue at REST side with an extra condition.

The drawback other than making the query a little more complex, is that converting from address to publicKey requires an extra trip to the database, the advantage is that since this is the signer we are talking about, we can be sure that the publicKey will be available. Note that clients cannot provide both signerPublicKey and address, avoiding extra trouble from param management.

Important note: this is not 100% perfect with this approach, because the issue goes a bit further than this, since we are only considering the signerPublicKey, and for different aggregate types we would still be skipping transactions with fields such as recipientAddress, targetAddress, etc... we could start adding OR to the conditions with all the extra address possibilities, but still seems hacky and error prone, so more discussion is probably still due

@Vektrat Vektrat added bug Something isn't working help wanted Extra attention is needed question labels Jul 24, 2020
@Vektrat Vektrat self-assigned this Jul 24, 2020
@Vektrat Vektrat changed the title Aggregate transactions cannot be filtered by address (instead of signerPublicKey) Embedded transactions cannot be filtered by address (instead of signerPublicKey) Jul 24, 2020
@Vektrat Vektrat removed the priority label Jul 31, 2020
@Vektrat
Copy link
Contributor Author

Vektrat commented Oct 9, 2020

@rg911 rg911 added the P3 Issue Low severity cosmetic issues with minor inconvenience label Feb 5, 2021
@rg911 rg911 removed the question label Feb 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed P3 Issue Low severity cosmetic issues with minor inconvenience
Projects
None yet
Development

No branches or pull requests

3 participants