-
Notifications
You must be signed in to change notification settings - Fork 38
add Account domain and implement redux-saga #244
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
Conversation
Explain line of code (browser.browserAction.openPopup() exists in Firefox but not in Chrome)
Taxi daemon via REST is returning assetAmount and assetSpread as string instead of number. Temporary fix, waiting for this issue to be fixed: vulpemventures/taxi-daemon#91
Note: clicking Receive and then on asset always worked
src/application/utils/transaction.ts
Outdated
if (!utxo) { | ||
throw new Error(`blindPSET error: utxo not found '${input.hash.reverse().toString('hex')}'`); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be occasions where the blinding is either a) not done by us (ccoperative blinding) b) not blinded (like taxi transactions for example)
for a) it's fine to wait new PSETv2 specification that cleanly allows to do this, but for b) we must skip throwing error for unblinded inputs we do not control (ie. provided by taxi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick note: this code makes impossible to use Taxi, since first input from psetToUnsignedTx is from a utxo the wallet doesn't have (it comes from Taxi), so it will always throw in the case of a Taxi transaction.
I remove it and tested all type of transactions, and everything is working.
Just not sure if by removing it I'm forgetting any edge case. @louisinger ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed throwing an error is not OK in case of unconfidential inputs (taxi especially). However, in case of confidential input we don't own, blinding will fail if we are not able to give it the blinders (utxo.blindingData).
Now, it's not a problem cause we always use Marina's inputs except the taxi ones but they are unconfidential. if in the future marina integrates a feature that uses confidential inputs from another wallet, it will be necessary to modify this function (add a parameter "externalInputsBlindingPrivateKeys" for instance). But I think it's better to wait for PSETv2 for this case as suggested by tiero (which would allow not to manage the blinding keys / blinders exchange).
TL;DR @bordalix no edge cases because Marina never handle cases of confidential inputs owned by another wallet. ACK to remove the check for me 👍
EDIT: I've pushed some comments on this function
…put would have an utxo that doesn't belong to wallet
…p from the stack
This happens with me when I'm sending a Tx to blockstream that:
Waiting for 2 minutes should work, since it gives time for previous Tx to confirm and Marina to update list of utxos and Tx. Having said that, I will raise the HTTP error to the UI. |
was the first tx I made since long time, yes raising the full explorer error would be best |
This PR is a kind a "rebase" of #238. The idea is to isolate the domain refacto + redux-saga from 2of2 multisig account logic .
Account domain
IdentityInterfaces
needed to (1) fetch utxos/txs and (2) sign transactions.Redux-saga
redux-thunk
andbackground-aliases
(from webext-redux).Bug fix
-Infinity
: now this is handled by theincrement
function in wallet reducer).it closes #242
it closes #204
@bordalix @tiero please review