-
Notifications
You must be signed in to change notification settings - Fork 19
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
Feature: on_social_set callback #18
base: master
Are you sure you want to change the base?
Conversation
@evgenykuzyakov By the way, since social-db is not up-to-date with the latest rust sdk I used an older callback syntax. Let me know if I there is interest in upgrading to latest version. I'd be happy to create a PR for it. |
near_sdk::env::block_height(), | ||
callback_receiver_id.clone(), | ||
0, | ||
Gas(5 * TGAS), |
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.
Let's let users specify the callback_attached_gas
(optional; default to 5 TGas)
contract/src/lib.rs
Outdated
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.
Let's revert unnecessary changes
|
||
callback_contract::on_social_set( | ||
near_sdk::env::block_height(), | ||
callback_receiver_id.clone(), |
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.
Why do you need to clone it?
let callback_receiver_id = options.callback_receiver_id.unwrap(); | ||
|
||
callback_contract::on_social_set( | ||
near_sdk::env::block_height(), |
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.
For DevHub's purposes, to inform which account posted the discussion, we also need a list of updated SocialDB keys OR we should be able to pass free-form params through set(options: { callback_params: {account_id} })
.
Getting the list of keys could be a bit of an involved process that would burn even more gas. Yet, free-form params can have security implications. Thus, I gravitate towards either passing the whole data
or passing only the list of keys.
fix: unnecessary unwrap Co-authored-by: Vlad Frolov <[email protected]>
At Devhub we could benefit from having an option to ask set method in SocialDB to make a cross-contract call to another contract to inform about the change. The use-case - we want to be able to make a post through NEAR Social (
<account>/post/main
) and trigger DevHub's contract, so it reshares the created post (we need block height and the updated keys to be provided from SocialDB contract). As mentioned by @frol hereI extended the set function with a callback and added callback_receiver_id to setoptions.
@evgenykuzyakov I wanted to solicit your feedbank. I implemented it so that the the receiver method is fixed as you mentioned. If you have any additional considerations or suggestions, please let me know. I'll incorporate them into the implementation, write a test case and remove the draft status from the PR.