anoma / namada

Rust implementation of Namada, a Proof-of-Stake L1 for interchain asset-agnostic privacy
https://namada.net
GNU General Public License v3.0
2.4k stars 951 forks source link

Amount to transfer is 1000000 when unshielding with `disposable-gas-payer` but without `gas-spending-key` #3961

Open Rigorously opened 2 days ago

Rigorously commented 2 days ago

Using v0.44.1-5-g7054446 tiago/backport-murisi-masp-fixes

namadac unshield   --source sp4415   --target namada-1-wallet   --token nam   --amount 1  --disposable-gas-payer
Enter your decryption password for sp4415: 
Created disposable keypair with alias disposable-key-814A253FC373AEE5D723DC3D56C9AE124145B216-created-at-1730222686
Error: 
   0: The balance of the source tnam1pcqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqzmefah is lower than the amount to be transferred. Amount to transfer is 1000000 tnam1qxgfw7myv4dh0qna4hq0xdg6lx77fzl7dcem8h7e
   1: The balance of the source tnam1pcqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqzmefah is lower than the amount to be transferred. Amount to transfer is 1000000 tnam1qxgfw7myv4dh0qna4hq0xdg6lx77fzl7dcem8h7e

Maybe the gas unit in the error has changed to micronam?

Nevertheless, the source owns 96.75 nam, so the error should not be triggered in the first place.

Rigorously commented 2 days ago

After adding --gas-spending-key, unshielding fails with namadac v0.44.1-5-g7054446 tiago/backport-murisi-masp-fixes too. https://github.com/anoma/namada/issues/3955

namadac unshield   --source sp4415   --target namada-1-wallet   --token nam   --amount 1  --disposable-gas-payer --gas-spending-key sp4415
Enter your decryption password for sp4415: 
Created disposable keypair with alias disposable-key-708029D339523892A428659DC45E0D48A882D9C8-created-at-1730223658
converting current asset type to latest asset type...
Error: 
   0: Encountered error while broadcasting transaction: StringTracer: server error: {"codespace":"","code":9,"data":"","log":"Mempool validation failed: Error trying to apply a transaction: Error while processing transaction's fees: Failed masp fee payment","hash":"7CDD760BA1928973997473B79E3567435D46BDED5F82D1B0F591E4ACDE5BAB98"}
   1: Encountered error while broadcasting transaction: StringTracer: server error: {"codespace":"","code":9,"data":"","log":"Mempool validation failed: Error trying to apply a transaction: Error while processing transaction's fees: Failed masp fee payment","hash":"7CDD760BA1928973997473B79E3567435D46BDED5F82D1B0F591E4ACDE5BAB98"}

And then without --gas-spending-key there is a new error:

 namadac unshield   --source sp4415   --target namada-1-wallet   --token nam   --amount 1  --disposable-gas-payer                          
Enter your decryption password for sp4415: 
Created disposable keypair with alias disposable-key-C257B85D37A59089D9E251481AFDFAE19BCCBC88-created-at-1730223692
Error: 
   0: Insufficient funds
   1: Insufficient funds
grarco commented 2 days ago

@Rigorously thanks for the report! It seems like we removed the denomination from a few internal types. I believe this is leading to the issues you are experiencing, we'll try to investigate more in depth

grarco commented 1 day ago

So from murisi/fix-masp-change I was able to reproduce your first issue. This happens because we now require the --gas-spending-key to be provided all the times when doing masp fee payment. We can put some constraints on the cli args so that this is more clear.

The third issue you encountered I was able to reproduce by forgetting to shielded-sync after a successful transfer, could this also be your case?

The second issue you encountered I was instead not able to reproduce. The error message you get comes from a node, so your client was indeed able to produce the tx but it was rejected when evaluated in mempool. There's a chance that error (even though quite generic) could refer to a gas error: could you retry by increasing the gas limit of your tx to something like 300k? If this is not the case then the client just constructed an invalid tx.

EDIT

Actually, never mind the last suggestion on the gas limit. We have a gas limit protocol param for masp fee payment, so you're likely bounded by that. If you are running a localnet you could try increasing that limit in the genesis/localnet/parameters.toml file, the param is called masp_fee_payment_gas_limit

Rigorously commented 1 day ago

So from murisi/fix-masp-change I was able to reproduce your first issue. This happens because we now require the --gas-spending-key to be provided all the times when doing masp fee payment. We can put some constraints on the cli args so that this is more clear.

Alright, makes sense. Reminds me of cases elsewhere where the price of a sold-out product is set to an exorbitant level to prevent people from buying it. But I wonder, what is the rationale behind making gas-spending-key mandatory?

I was just asking myself why unshielding should not be done with a disposable-gas-payer and the source as gas-spending-key by default. Then the command becomes simply unshield --source $SPENDING_KEY --target $TRANSPARENT_ACCOUNT --token $TOKEN --amount $AMOUNT. Seems safer, easier and intuitive.

The docs give the impression that using an implicit account as gas-payer is recommended and disposable-gas-payer is an advanced technique. In practice, using an implicit account to pay the unshielding fees carries the risk of accidentally linking your transparent account to the shielded transaction. Requiring gas-spending-key makes the use of disposable-gas-payer look even more awkward.

The third issue you encountered I was able to reproduce by forgetting to shielded-sync after a successful transfer, could this also be your case?

I added it because the previous transactions failed. How can there be a different error if the transaction failed? Maybe something changed on the client side, while the network rejected it. I hoped this third observation could give clues to solve the underlying problem.

There's a chance that error (even though quite generic) could refer to a gas error: could you retry by increasing the gas limit of your tx to something like 300k? If this is not the case then the client just constructed an invalid tx.

Good idea! I will try that.

Rigorously commented 1 day ago

Instead of increasing the gas limit, I did the opposite.

I just had a successful unshielding, then synced, then tried unshielding again with --gas-limit 100. It causes a different error. Therefore, the second issue (which is https://github.com/anoma/namada/issues/3955) is not caused by running out of gas.

 Error: 
   0: Encountered error while broadcasting transaction: StringTracer: server error: {"codespace":"","code":8,"data":"","log":"{INVALID_MSG}: Wrapper transaction exceeds its gas limit","hash":"21D99052C4DB25516733F4C00F8843ADD96E028FB799AECEF9311AB1F61BCADF"}
   1: Encountered error while broadcasting transaction: StringTracer: server error: {"codespace":"","code":8,"data":"","log":"{INVALID_MSG}: Wrapper transaction exceeds its gas limit","hash":"21D99052C4DB25516733F4C00F8843ADD96E028FB799AECEF9311AB1F61BCADF"
grarco commented 20 hours ago

Ok good so it's not a gas issue. I'll try to investigate further on a local network since the debug information we need is only emitted in the node.

To go back to you previous post:

  1. Agree with you, in the attempt to fix a bug in the SDK function that generate MASP transactions we've simplified the algorithm leading to some loss in terms of UX. Both your points though are valid: gas-payer was more of a test argument that should not be allowed (or at least no encouraged) and we should also be able to infer the gas payer by just using the same spending key passed in as the source. I'll open an issue for these
  2. The error can differ between submissions because under the hood we invalidate output notes in your shielded wallet when you submit a transaction. This was intended to reduce the amount of calls to the shielded-sync command and also as a way to allow for gas payment via the MASP with an old logic that is no more (and which would have not worked without this trick). Unfortunately the logic invalidates the notes that you've used in the transaction right after it's done constructing it, so without subscribing to the actual result of the tx. This means that if your tx is successful than everything will be ok, but if it fails we invalidate notes that you could in theory spend (since they've never been nullified in protocol), leading to the error message you are seeing. We have an issue about this here #2593