-
Notifications
You must be signed in to change notification settings - Fork 101
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
MASP indexer integration #1120
MASP indexer integration #1120
Conversation
5828d30
to
ebb576d
Compare
built_txs.push(tx); | ||
} | ||
|
||
// Get wrapper args | ||
let first_tx = built_txs | ||
.get(0) | ||
.first() |
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.
thanks, I thought I had updated all of these :D
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.
I've recently setup clippy, it shows all that stuff :D
@@ -761,6 +765,8 @@ pub fn ibc_transfer_tx_args( | |||
channel_id, | |||
timeout_height, | |||
timeout_sec_offset, | |||
// TODO: false for now | |||
disposable_signing_key: false, |
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.
What do you think about mapping this to the types/args before we release? I could do a follow-up PR that adds any potential args we may want soon to our types package, that way we wouldn't have to worry about any breaking APIs after we publish. Unless there's not a use-case we'll need for this. I can look into that!
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.
I think we can add it when we start testing transfers
ebb576d
to
e575072
Compare
@@ -124,11 +124,11 @@ mod tests { | |||
.expect("Deriving from ExtendedKeys should not fail"); | |||
|
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.
Weird, I'm now getting a build warning on this file, though not sure why it's happening on this branch:
warning: field `0` is never read
--> src/crypto/zip32.rs:15:27
|
15 | pub struct ExtSpendingKey(pub(crate) ExtendedSpendingKey);
| -------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| field in this struct
Not sure that we're using that type any more. Not a big deal on this PR, just noting!
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.
Code LGTM! Was able to test this locally, works great! I think it's fine to point to that rev
for now, we can later point to the next tag that has the fixes we need.
I don't think there is an easy way to test this for now :/ But I think as long as this does not break other features we can merge it.
Okay!
Make sure you test against 0.44.0
So to test shielded sync w/o masp indexer you can:
target/debug/namada wallet gen --shielded --alias albert-shielded --base-dir .namada/validator-0 target/debug/namada wallet gen-payment-addr --alias albert-pa1 --key albert-shielded --base-dir .namada/validator-0 target/debug/namada client shield --source albert --target albert-pa1 --token nam --amount 1000 --base-dir .namada/validator-0 # To get the viewing key target/debug/namada wallet find --alias albert-shielded --base-dir .namada/validator-0
App.tsx
in namadillo(with missing imports)await window.shieldedSync()
in namadillo console, you should see progress loggedTo test with masp indexer you can do the same setup but also run masp_indexer locally.
❗CLEAR CONTEXT MANUALLY BETWEEN TESTS, JUST REMOVE IT FROM IDB ❗
ALSO currently I;m getting this :(