-
Notifications
You must be signed in to change notification settings - Fork 969
XLS-101: Smart Contracts #406
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
base: master
Are you sure you want to change the base?
Conversation
| | `Owner` | ✔️ | `string` | `AccountID` | The owner of the contract, which defaults to the account that deployed the contract. | | ||
| | `Flags` | ✔️ | `number` | `UInt32` | Flags that may be on this ledger entry. | | ||
| | `Sequence` | ✔️ | `string` | `UInt16` | The ledger entry's type (`ContractSource`). | | ||
| | `ContractHash` | ✔️ | `string` | `Hash256` | The hash of the contract's code. | |
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.
Contract Hash or the hash of prefix + ContractHash?
We cannot link them in the explorer very easily if we do just wasm hash
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.
Or add ContractSourceID for easy linking?
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.
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.
I was thinking the hash directly, but I don't think I have a strong opinion on this - so fine with ContractSourceID
| | :---------------- | :-------- | :-------- | :------------ | :------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `TransactionType` | ✔️ | `string` | `UInt16` | The transaction type (`ContractUserDelete`). | | ||
| | `Account` | ✔️ | `string` | `AccountID` | The account sending the transaction (the user). | | ||
| | `ContractAccount` | | `string` | `AccountID` | The pseudo-account hosting the contract that is to be changed. This field is not needed if the pseudo-account is emitting this transaction itself. | |
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.
Does the contract really need to call this? Would a host api be better? Emitting a transaction to do this seems inefficient.
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.
Yeah that's fair. Should we add this to this list of transaction types that can't be emitted, then?
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.
Yeah that's fair. Should we add this to this list of transaction types that can't be emitted, then?
Yepp good point
|
|
||
| #### 4.1.1. Object ID | ||
|
|
||
| hash of prefix + `ContractHash` + `Sequence` |
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.
Note: Original ContractHash + Sequence. The actual contract hash may change through ContractModify transactions
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.
Should we go back to Owner + Sequence? Just needs to be something, right? Now that I think about it, ContractHash + Sequence could result in a hash collision if two accounts with the same sequence number create the same contract.
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.
it is, in the code now its hash, owner, seq. I'm fine with owner + seq.
| - Possible flags: | ||
| - `tfSendAmount` - if the type is an `STAmount`, then that amount will be sent to the contract from your funds | ||
| - `tfSendNFToken` - if the type is a `Hash256`, then the `NFToken` with that ID will be sent to the contract from your holdings | ||
| - `tfAuthorizeTokenHolding` - if the type is an `STIssue` or `STAmount`, then you can automatically create a trustline/MPToken for that token (assuming it’s not XRP). |
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.
Doing this on ContractCall could create an exploit. If this is allowed then I could create 100's of trustlines on the contract. Either we limit to Create/Modify or the contract itself just authorizes the trustline through an init function. (Imo authorize in the contract is a better route)
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.
I don't quite follow - this is the user authorizing holding a token, not the contract. The contract would be the issuer in this case. And you're only authorizing that one token.
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.
If you want the contract to be able to hold a token, the contract needs to add the trust line. I think I realize now thats not what you meant by this?
You mean if the contract creates an IOU/MPT that requires auth then this authorizes the user? Why wouldn't the user just use SetTrust or MPTAuthorize?
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.
You mean if the contract creates an IOU/MPT that requires auth then this authorizes the user? Why wouldn't the user just use SetTrust or MPTAuthorize?
Yeah, the user automatically authorizes the token. Just saves you one step and makes it a bit more of a seamless experience is all, IMO. I figured there would be some need to "approve" holding a token, but it felt clunky to have that separate/just indicate a number of trustlines/MPTs you'd be willing to hold - this felt the cleanest to me.
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.
Ahh right, and we know that this contract uses an IOU/MPT and you must authorize first.
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.
Yeah - like an LP token.
No description provided.