-
Notifications
You must be signed in to change notification settings - Fork 16
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
Update Fees Handling part in xcall #55
Conversation
@@ -134,7 +134,7 @@ before the call request. | |||
|
|||
If a DApp needs to handle a rollback operation, it would fill some data in the last `_rollback` parameter of the `sendCallMessage`, | |||
otherwise it would have a null value which indicates no rollback handling is required. | |||
When an error occurs and the `_rollback` is not null, `xcall` emits the following event for notifying the user. | |||
When an error occurs and the `_rollback` is not null, `xcall` emits the following event for notifying the user on the source chain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this being enforced?
Note that the `executeRollback` can be called only when the original call request has responded with a failure.
Is there a verification somehow that associates _sn
with a failed message on destination chain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTP is a layered architecture, so the assumption here is that the failure message delivery is guaranteed by underlying BMC and Relay operations. xcall
service just needs to define its own protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that protocol defined here? This would be the place to define the xcall service protocol, no?
IIPS/iip-52.md
Outdated
depend on the destination network address. Also, a `default` fee has been defined in case of there is no fee | ||
mapping to the network address. | ||
If a user wants to make a call from ICON to the T1 network, he needs to pay X ICX, and for the T2 network | ||
he needs to pay Y ICX. That is, the fees depend on the destination network address. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any other insight into how the fee is determined? Additionally, why is the Fee Handling
section being discussed in the IIP for ICON BTP Arbitrary Call Service Standard
? Are these fees specific to the xcall standard, or are they a more general topic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an outcome of long discussion between our devs and the Foundation members (Scott, Hong and TJ).
We are thinking xcall
would be the only service using the BTP protocol in the end, so fee handling is the key functional part of the service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As BTP is an open framework, there are no rules that enforce that someone else who implements BTP could not define a separate service. For example, a different arbitrary call service (e.g. arbCall) with different rules, but with the same fee structure
These two pieces of information are sufficiently separate that I believe it should be a separate IIP. Something like the following:
IIP: BTP Service Fee Handling
Here are getter and setter methods for the proper fees handling in `xcall`. | ||
DApps that want to make a call to `sendCallMessage`, should query the total fee amount for the destination | ||
network via `getFee` interface, and then enclose the appropriate fees in the method call. | ||
Note that the protocol fee amount can be get/set via `xcall`, but the relay fee would be obtained from BMC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add the name of the method for getting/setting the fee?
*/ | ||
@External(readonly=true) | ||
BigInteger fixedFee(String _net, String _type); | ||
BigInteger getFee(String _net, boolean _rollback); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some information about how the commission is related to the presence or absence of the rollback parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rollback
requires two-way BTP message passing. That is, one for the requested message itself and the other one for the result processed in the destination chain. So if the user need to handle rollback, the user needs to pay additionally for the result message passing.
|
||
/** | ||
* Gets the total fixed fees for the given network address. | ||
* If there is no mapping to the network address, `default` fee is returned. | ||
* Sets the protocol fee amount. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe in more detail what a protocol commission is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it was an request from @BennyOptions , see icon-project/btp-product#46.
The accrued protocol fees on each chain could be auctioned in exchange for ICX, and this process is another use case of BTP, then eventually could value up of ICX.
For the auction system, see the discussion icon-project/btp#65.
You may ask @BennyOptions for further behind rationale about this.
Discussion thread in #52