Skip to content
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

Murisi/masp separate parallel sync #2474

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Separated MASP syncing functionality into a separate subcommand.
([\#2474](https://github.com/anoma/namada/pull/2474))
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/2458-masp-scanning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Simplified the transaction fetching algorithm to enable it to be saved to
storage more frequently. ([\#2458](https://github.com/anoma/namada/pull/2458))
3 changes: 2 additions & 1 deletion crates/apps/src/lib/bench_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -958,8 +958,9 @@ impl BenchShieldedCtx {
.find_spending_key(ALBERT_SPENDING_KEY, None)
.unwrap();
async_runtime
.block_on(self.shielded.fetch(
.block_on(self.shielded.sync(
&self.shell,
None,
&[spending_key.into()],
&[],
))
Expand Down
68 changes: 68 additions & 0 deletions crates/apps/src/lib/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ pub mod cmds {
.subcommand(QueryMetaData::def().display_order(5))
// Actions
.subcommand(SignTx::def().display_order(6))
.subcommand(ShieldedSync::def().display_order(6))
.subcommand(GenIbcShieldedTransafer::def().display_order(6))
// Utils
.subcommand(Utils::def().display_order(7))
Expand Down Expand Up @@ -346,6 +347,7 @@ pub mod cmds {
let add_to_eth_bridge_pool =
Self::parse_with_ctx(matches, AddToEthBridgePool);
let sign_tx = Self::parse_with_ctx(matches, SignTx);
let shielded_sync = Self::parse_with_ctx(matches, ShieldedSync);
let gen_ibc_shielded =
Self::parse_with_ctx(matches, GenIbcShieldedTransafer);
let utils = SubCmd::parse(matches).map(Self::WithoutContext);
Expand Down Expand Up @@ -397,6 +399,7 @@ pub mod cmds {
.or(query_metadata)
.or(query_account)
.or(sign_tx)
.or(shielded_sync)
.or(gen_ibc_shielded)
.or(utils)
}
Expand Down Expand Up @@ -483,6 +486,7 @@ pub mod cmds {
QueryValidatorState(QueryValidatorState),
QueryRewards(QueryRewards),
SignTx(SignTx),
ShieldedSync(ShieldedSync),
GenIbcShieldedTransafer(GenIbcShieldedTransafer),
}

Expand Down Expand Up @@ -1344,6 +1348,29 @@ pub mod cmds {
}
}

#[derive(Clone, Debug)]
pub struct ShieldedSync(pub args::ShieldedSync<args::CliTypes>);

impl SubCmd for ShieldedSync {
const CMD: &'static str = "shielded-sync";

fn parse(matches: &ArgMatches) -> Option<Self> {
matches
.subcommand_matches(Self::CMD)
.map(|matches| ShieldedSync(args::ShieldedSync::parse(matches)))
}

fn def() -> App {
App::new(Self::CMD)
.about(
"Sync the local shielded context with MASP notes owned by \
the provided viewing / spending keys up to an optional \
specified block height.",
)
.add_args::<args::ShieldedSync<args::CliTypes>>()
}
}

#[derive(Clone, Debug)]
pub struct Bond(pub args::Bond<args::CliTypes>);

Expand Down Expand Up @@ -3095,6 +3122,8 @@ pub mod args {
pub const VALUE: Arg<String> = arg("value");
pub const VOTER_OPT: ArgOpt<WalletAddress> = arg_opt("voter");
pub const VIEWING_KEY: Arg<WalletViewingKey> = arg("key");
pub const VIEWING_KEYS: ArgMulti<WalletViewingKey, GlobStar> =
arg_multi("viewing-keys");
pub const VP: ArgOpt<String> = arg_opt("vp");
pub const WALLET_ALIAS_FORCE: ArgFlag = flag("wallet-alias-force");
pub const WASM_CHECKSUMS_PATH: Arg<PathBuf> = arg("wasm-checksums-path");
Expand Down Expand Up @@ -5601,6 +5630,45 @@ pub mod args {
}
}

impl Args for ShieldedSync<CliTypes> {
fn parse(matches: &ArgMatches) -> Self {
let ledger_address = LEDGER_ADDRESS_DEFAULT.parse(matches);
let last_query_height = BLOCK_HEIGHT_OPT.parse(matches);
let viewing_keys = VIEWING_KEYS.parse(matches);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not want to support passing in spending keys here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whenever a spending key is added to the wallet, its corresponding viewing key is also added under the same name. So I'm thinking that that this code should be sufficient for now. No?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I just wondered if we considered it important.

Self {
ledger_address,
last_query_height,
viewing_keys,
}
}

fn def(app: App) -> App {
app.arg(LEDGER_ADDRESS_DEFAULT.def().help(LEDGER_ADDRESS_ABOUT))
.arg(BLOCK_HEIGHT_OPT.def().help(
"Option block height to sync up to. Default is latest.",
))
.arg(VIEWING_KEYS.def().help(
"List of new viewing keys with which to check note \
ownership. These will be added to the shielded context.",
))
}
}

impl CliToSdk<ShieldedSync<SdkTypes>> for ShieldedSync<CliTypes> {
fn to_sdk(self, ctx: &mut Context) -> ShieldedSync<SdkTypes> {
let chain_ctx = ctx.borrow_mut_chain_or_exit();
ShieldedSync {
ledger_address: (),
last_query_height: self.last_query_height,
viewing_keys: self
.viewing_keys
.iter()
.map(|vk| chain_ctx.get_cached(vk))
.collect(),
}
}
}

impl CliToSdk<GenIbcShieldedTransafer<SdkTypes>>
for GenIbcShieldedTransafer<CliTypes>
{
Expand Down
33 changes: 33 additions & 0 deletions crates/apps/src/lib/cli/client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use color_eyre::eyre::Result;
use masp_primitives::sapling::ViewingKey;
use masp_primitives::zip32::ExtendedFullViewingKey;
use namada::types::io::Io;
use namada_sdk::{Namada, NamadaImpl};

Expand Down Expand Up @@ -298,6 +300,37 @@ impl CliApi {
tx::submit_validator_metadata_change(&namada, args)
.await?;
}
Sub::ShieldedSync(ShieldedSync(mut args)) => {
let client = client.unwrap_or_else(|| {
C::from_tendermint_address(&mut args.ledger_address)
});
client.wait_until_node_is_synced(&io).await?;
let args = args.to_sdk(&mut ctx);
let namada = ctx.to_sdk(client, io);
let mut vks: Vec<ViewingKey> = namada
.wallet()
.await
.get_viewing_keys()
.values()
.copied()
.map(|vk| ExtendedFullViewingKey::from(vk).fvk.vk)
.collect();
vks.extend(
args.viewing_keys.into_iter().map(|vk| {
ExtendedFullViewingKey::from(vk).fvk.vk
}),
);
let mut shielded = namada.shielded_mut().await;
let _ = shielded.load().await;
shielded
.sync(
namada.client(),
args.last_query_height,
&[],
&vks,
)
.await?;
}
// Eth bridge
Sub::AddToEthBridgePool(args) => {
let mut args = args.0;
Expand Down
12 changes: 1 addition & 11 deletions crates/apps/src/lib/client/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,7 @@ pub async fn query_transfers(
let _ = shielded.precompute_asset_types(context).await;
// Obtain the effects of all shielded and transparent transactions
let transfers = shielded
.query_tx_deltas(
context.client(),
&query_owner,
&query_token,
&wallet.get_viewing_keys(),
)
.query_tx_deltas(context.client(), &query_owner, &query_token)
.await
.unwrap();
// To facilitate lookups of human-readable token names
Expand Down Expand Up @@ -878,11 +873,6 @@ pub async fn query_shielded_balance(
{
let mut shielded = context.shielded_mut().await;
let _ = shielded.load().await;
let fvks: Vec<_> = viewing_keys
.iter()
.map(|fvk| ExtendedFullViewingKey::from(*fvk).fvk.vk)
.collect();
shielded.fetch(context.client(), &[], &fvks).await.unwrap();
// Precompute asset types to increase chances of success in decoding
let _ = shielded.precompute_asset_types(context).await;
// Save the update state so that future fetches can be short-circuited
Expand Down
1 change: 1 addition & 0 deletions crates/core/src/types/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1463,6 +1463,7 @@ impl<E> GetEventNonce for InnerEthEventsQueue<E> {
PartialEq,
Ord,
PartialOrd,
Hash,
)]
pub struct IndexedTx {
/// The block height of the indexed tx
Expand Down
15 changes: 14 additions & 1 deletion crates/sdk/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use namada_core::types::ethereum_events::EthAddress;
use namada_core::types::keccak::KeccakHash;
use namada_core::types::key::{common, SchemeType};
use namada_core::types::masp::PaymentAddress;
use namada_core::types::storage::Epoch;
use namada_core::types::storage::{BlockHeight, Epoch};
use namada_core::types::time::DateTimeUtc;
use namada_core::types::{storage, token};
use namada_governance::cli::onchain::{
Expand Down Expand Up @@ -1823,6 +1823,19 @@ pub struct SignTx<C: NamadaTypes = SdkTypes> {
pub owner: C::Address,
}

#[derive(Clone, Debug)]
/// Sync notes from MASP owned by the provided spending /
/// viewing keys. Syncing can be told to stop at a given
/// block height.
pub struct ShieldedSync<C: NamadaTypes = SdkTypes> {
/// The ledger address
pub ledger_address: C::TendermintAddress,
/// Height to sync up to. Defaults to most recent
pub last_query_height: Option<BlockHeight>,
/// Viewing keys used to determine note ownership
pub viewing_keys: Vec<C::ViewingKey>,
}

/// Query PoS commission rate
#[derive(Clone, Debug)]
pub struct QueryCommissionRate<C: NamadaTypes = SdkTypes> {
Expand Down
Loading
Loading