You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jan 13, 2023. It is now read-only.
pdecol
wants to merge
1
commit into
iotaledger:develop
from
pdecol:feature/217-snapshot-proof-commands
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR makes getNewAddresses, getInputs, getTransfers and getAccountData snapshot proof. It also follows the behaviour of the Javascript library more closely but it is not exactly the same. There is a difference in how these commands behave on addresses with balances and without transactions. I opened a ticket in the JavaScript GitHub repository where I documented the behaviour of the JavaScript library on such addresses (iotaledger/iota.js#427). As I stated in this ticket I think that the behaviour of the JavaScript library is in these cases a bit strange and I think that my suggestion that I also implemented in this PR is more logical. There has been no response so far but maybe @lzpap could help to coordinate how the commands should behave for addresses with balance and without transactions?
hey @pdecol, awesome! I'm on it ;)
However, bear in mind that this might be low priority for the other libs that already have account module, which solves the after snapshot issue by making the lib (locally) stateful.
Looking at Chris's comment (iotaledger/iota.js#427 (comment)), I'm wondering if we should hold off on this change until we implement the account module in PyOTA — it seems like the changes needed to implement that will conflict with what's being proposed in this PR. @pdecol@lzpap What do you think?
What I try to clarify in the other ticket is only a part of what is proposed in this PR. To be precise it is about whether or not the getBalances API call should be added to getNewAddresses and iter_used_addresses. By doing so the issues that I mentioned in the other ticket would be solved.
The other changes (adding the wereAddressesSpentFrom API call to getNewAddresses and iter_used_addresses and the change in get_account_data.py) are currently implemented that way in the JavaScript library and are needed to establish an identical behaviour between the two libraries.
I suggest then that we break up the PR into the two changes that you mentioned @pdecol , and put the first one on hold until we have a concept about account module in pyota.
The second change is indeed needed to establish identical behavior, I agree.
Thanks for the proactive approach @pdecol! :)
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves #217, resolves #175, resolves #46
This PR makes getNewAddresses, getInputs, getTransfers and getAccountData snapshot proof. It also follows the behaviour of the Javascript library more closely but it is not exactly the same. There is a difference in how these commands behave on addresses with balances and without transactions. I opened a ticket in the JavaScript GitHub repository where I documented the behaviour of the JavaScript library on such addresses (iotaledger/iota.js#427). As I stated in this ticket I think that the behaviour of the JavaScript library is in these cases a bit strange and I think that my suggestion that I also implemented in this PR is more logical. There has been no response so far but maybe @lzpap could help to coordinate how the commands should behave for addresses with balance and without transactions?