Too wide suggested ranges for Testnet

[HonzaR]

Hi, when testing the new SbS sync algorithm with our mobile SDKs, we noticed that the Testnet server (lightwalletd.testnet.electriccoin.co) replies to suggestScanRanges besides others with also very old blocks for a new wallet: ScanRange(range=BlockHeight(value=490074)..BlockHeight(value=2459999), priority=ChainTip). My guess is the getSubtreeRoots. Here is my getSubtreeRoot args:

downloader.getSubtreeRoots(
   startIndex = 0,
   maxEntries = if (network.isTestnet()) {
      65536
   } else {
      0
   },
   shieldedProtocol = ShieldedProtocolEnum.SAPLING
)

Here is a log for the SbS syncing:

CompactBlockProcessor.start$suspendImpl(): Fetched SubTreeRoot result: UseSbS(subTreeRootList=[SubtreeRoot(completingBlockHeight=490074)]), with preferred sync algorithm: SPEND_BEFORE_SYNC
CompactBlockProcessor.processNewBlocksInSbSOrder(): Beginning to process new blocks with Spend-before-Sync approach (with roots: [SubtreeRoot(completingBlockHeight=490074)], and lowerbound: BlockHeight(value=2459900))...
CompactBlockProcessor.putSaplingSubtreeRoots$zcash_android_sdk_1_20_0_beta01_release(): Sapling subtree roots put successfully with startIndex: 0 and roots: 1
CompactBlockProcessor.updateChainTip$zcash_android_sdk_1_20_0_beta01_release(): Chain tip updated successfully with height: BlockHeight(value=2469481)
CompactBlockProcessor.suggestScanRanges$zcash_android_sdk_1_20_0_beta01_release(): Successfully got newly suggested ranges: [ScanRange(range=BlockHeight(value=2460000)..BlockHeight(value=2460009), priority=Verify), ScanRange(range=BlockHeight(value=2460010)..BlockHeight(value=2469481), priority=ChainTip), ScanRange(range=BlockHeight(value=490074)..BlockHeight(value=2459999), priority=ChainTip)]
CompactBlockProcessor.processNewBlocksInSbSOrder(): Check for verification of ranges resulted with: ShouldVerify(scanRange=ScanRange(range=BlockHeight(value=2460000)..BlockHeight(value=2460009), priority=Verify))
CompactBlockProcessor.processNewBlocksInSbSOrder(): Starting verification of range: ShouldVerify(scanRange=ScanRange(range=BlockHeight(value=2460000)..BlockHeight(value=2460009), priority=Verify))
CompactBlockProcessor.invokeSuspend(): Syncing blocks in range BlockHeight(value=2460000)..BlockHeight(value=2460009)

And here is a log of also a new wallet, which operates upon the Linear sync algorithm:

CompactBlockProcessor.processNewBlocksInLinearOrder(): Beginning to process new blocks with Linear approach (with lower bound: BlockHeight(value=2459900))...
CompactBlockProcessor.invokeSuspend(): Syncing blocks in range BlockHeight(value=2459901)..BlockHeight(value=2469486)

You can see that the Linear one syncs newer blocks than the SbS one, which goes much deeper. This seems to be Testnet-only related.

[str4d]

I do not know what the current deployment state of the ECC testnet lightwalletd is (what revision of lightwalletd is running, and what zcashd state). @yasserisa?

[y4ssi]

I do not know what the current deployment state of the ECC testnet lightwalletd is (what revision of lightwalletd is running, and what zcashd state). @yasserisa?

This is the config of the testnet node (5.6.1):

server=1
testnet=1
experimentalfeatures=1
lightwalletd=1
txindex=1
insightexplorer=1

and here the version of the lwd

 grpcurl lightwalletd.testnet.electriccoin.co:9067 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetLightdInfo
{
  "version": "v0.4.15",
  "vendor": "ECC LightWalletD",
  "taddrSupport": true,
  "chainName": "test",
  "saplingActivationHeight": "280000",
  "consensusBranchId": "c2d6d0b4",
  "blockHeight": "2473129",
  "gitCommit": "f7795c83a397dcb25fc2779537308fb91e1bc99d",
  "buildDate": "2023-07-07",
  "buildUser": "root",
  "estimatedHeight": "2473129",
  "zcashdBuild": "v5.6.1",
  "zcashdSubversion": "/MagicBean:5.6.1/"
}

please let me know if this setup is okay or if we need any changes!

[str4d]

lightwalletd should be updated, because that revision doesn't include the maxEntries bugfix (which would remove the need for the conditional in @HonzaR's code). However, other than that I don't see why this setup would be returning out-of-date blocks.

@HonzaR can you clarify what the problem is that you are seeing? If it's that the wallet tries to scan from height 490074 at all, I suspect it is a side-effect of testnet having so little traffic that we only have two subtrees:

• One complete subtree from Sapling activation (200,000) to 490,074.
• One incomplete subtree from 490,074 to the chain tip.

If that's what you're seeing, then this issue has the same root cause, and fix, as #902. The wallet on initialization needs to declare all blocks prior to the wallet birthday as "scanned", and instead insert the provided Sapling frontier into the ShardTree so it only needs to scan from the birthday height.

[y4ssi]

lightwalletd should be updated, because that revision doesn't include the maxEntries bugfix (which would remove the need for the conditional in @HonzaR's code). However, other than that I don't see why this setup would be returning out-of-date blocks.

@HonzaR can you clarify what the problem is that you are seeing? If it's that the wallet tries to scan from height 490074 at all, I suspect it is a side-effect of testnet having so little traffic that we only have two subtrees:

• One complete subtree from Sapling activation (200,000) to 490,074.
• One incomplete subtree from 490,074 to the chain tip.

If that's what you're seeing, then this issue has the same root cause, and fix, as #902. The wallet on initialization needs to declare all blocks prior to the wallet birthday as "scanned", and instead insert the provided Sapling frontier into the ShardTree so it only needs to scan from the birthday height.

I just updated the testnet lwd to the latest version (v0.4.16). Is that the version we need?

[HonzaR]

Hi @str4d, yes, I can confirm I still see this error. @yasserisa update doesn't seem to change anything in it. Also, if I use maxEntries = 0, I still get 490,074 as a lower bound of the last range (and subtree root fetch). IIUC you guys work on some changes which will address this on the librustzcash side.

[nuttycom]

This should be fixed by #907

[HonzaR]

@nuttycom We don't have permission to add dependencies for this ticket. Could you connect this to the #907?

[str4d]

Done