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

Network not set on invoice #103

Closed
cryptoquick opened this issue Aug 30, 2023 · 9 comments
Closed

Network not set on invoice #103

cryptoquick opened this issue Aug 30, 2023 · 9 comments
Labels
question Further information is requested

Comments

@cryptoquick
Copy link
Member

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.

@dr-orlovsky
Copy link
Member

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

@dr-orlovsky dr-orlovsky added the question Further information is requested label Sep 6, 2023
@nicbus
Copy link
Contributor

nicbus commented Sep 14, 2023

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.

@dr-orlovsky
Copy link
Member

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)?

@cryptoquick
Copy link
Member Author

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 trgb.

@dr-orlovsky
Copy link
Member

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:

rgb:contractId/RGB20?net=signet

@cryptoquick
Copy link
Member Author

That could work!

@nicbus
Copy link
Contributor

nicbus commented Sep 18, 2023

Having an optional query parameter seems better because it retains backwards compatibility and defaulting to mainnet when not provided should be fine.

@dr-orlovsky
Copy link
Member

Done my version in #104

@dr-orlovsky
Copy link
Member

The final solution had to combine both testnet info with the chain info (due to added support for Liquid and more future layer1).

#118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants