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

fill_order_operation depends on object numbering #570

Closed
theoreticalbts opened this issue Feb 12, 2016 · 3 comments
Closed

fill_order_operation depends on object numbering #570

theoreticalbts opened this issue Feb 12, 2016 · 3 comments

Comments

@theoreticalbts
Copy link
Contributor

We are supposed to keep the object database out of the protocol layer, but fill_order_operation includes an object_id_type. Maybe it's OK because it's a v-op, but that implies v-ops aren't part of the protocol layer -- however, they have slots in the protocol-level static_variant.

This is basically an overall symptom of needing to more clearly separate the project into modular pieces -- or refine and consistently use architectural separations that are being ignored by the implementation like #246.

@abitmore
Copy link
Contributor

Just a note here: the percentage based transfer fee feature requires an asset_object to be passed into the operation (protocol level) to be able to calculate fee. Any suggestions?

@theoreticalbts
Copy link
Contributor Author

@abitmore : In general, your operations should use an asset_id_type, then fetch the asset_object referred to by the ID in the evaluator.

This percentage transfer fee feature actually needs a great deal of review, as it breaks one of the design principles of the Graphene toolkit, which is that the fee for an operation can be determined only from the operation and the current fee schedule. Now transferring some assets will have a fee which cannot be determined from that information alone (as you'll need to know whether the asset issuer's enabled the percentage-based fee for their asset).

This BSIP also really doesn't "play nice" with the existing machinery which converts the fee. There's really no way for you to tell the blockchain "for this operation, don't convert the fee to BTS" -- it's already converted by the time the op knows about it.

I think the sanest way to do percentage-based transfer fees is to make them an extension on the transfer operation, which basically says "I'm including this extra BTS / asset to be paid according to the BSIP rules," and then have another extension in the asset_update_operation which allows an asset owner to say "transfer of my asset is disallowed unless the transfer op includes the extension with so much on it." Then the main fee's paid into network / referral system as normal and the BSIP is an extra fee can be handled by totally custom logic.

TLDR, use the extension system to add another field to the transfer op, instead of trying to jam it into the existing fee field and have the existing fee handling code get in your way (what you're doing is different enough that it will get in your way).

This whole comment probably belongs in a new ticket :)

@abitmore
Copy link
Contributor

New ticket #583 created, so I'll post replies of BSIP10 related questions there.

@vikramrajkumar
Copy link
Contributor

This issue was moved to bitshares/bitshares-core#156

pmconrad pushed a commit to pmconrad/graphene that referenced this issue Mar 11, 2018
`get_required_signatures` API returns owner keys, don't return active if also requires owner, don't return signed keys.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants