-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Network not set on invoice #103
Comments
The network is inside the contract id: contract genesis commits to the network. So on a wrong network it will be impossible to do a payment since the contract would not be found |
While a transfer would not complete successfully if the wrong network is used, IMO it's still better to have the network available in the invoice string. Also a wallet would be able to prevent a transfer right away, without the need to even start the transfer process. An advantage would be that a user could recognize that the network is the expected one just by looking at the invoice string, without the need to decode it (which requires the use of a software). An issue with the current state is that, when decoding an invoice string (with rgb-wallet), the network is present only for witness transfers (which derive it from the address) and is missing for blinded transfers (which have no info to derive it from). Another issue is that an invoice with no contract ID cannot provide network info, as that's only available in the contract. Finally, I think that with the current implementation there's the possibility for a bigger bug: a wallet could pay a blinded invoice that has no contract ID using assets for a different network from the one of the receiver wallet. In this case, the sender would immediately accept the transfer into its stash and potentially broadcast the transaction to the network. Even though the receiver would refuse the transfer, as the networks would be different and the transaction wouldn't show up in its resolver, if the sender already broadcasted the TX it would lose the assets. |
Makes sense. Any ideas for the best form for providing explicit network information in the current invoice format? Must it be made required (thus breaking backward compatibility of the past invoices)? |
Not required, most fields on the invoice struct are Option already, and so long as it can be deserialized in the struct, it doesn't need to be user-readable necessarily... Though, perhaps as a convention, testnet invoices could be prefixed with |
I think we can say that if the network is not provided it defaults to mainnet. I am not sure about using custom URL scheme for testnet since it increases complexity for wallet registration with OS. I rather propose query parameter:
|
That could work! |
Having an optional query parameter seems better because it retains backwards compatibility and defaulting to mainnet when not provided should be fine. |
Done my version in #104 |
The final solution had to combine both testnet info with the chain info (due to added support for Liquid and more future layer1). |
Despite setting the network argument, we get still invoices like this:
rgb:Cbw1h3zbHgRhA6sxb4FS3Z7GTpdj9MLb7Do88qh5TUH1/RGB20/1+utxob02FKN7XB4N5LWc1d6cXfkByZ8d1jB4BZhee1vdU7MwffTtLkiK3
When parsed, it lacks the network argument.
This will be helpful for users to better verify and troubleshoot their transfers.
The text was updated successfully, but these errors were encountered: