stacks-archive / stacks-transactions-js

The JavaScript library for generating Stacks 2.0 transactions
19 stars 17 forks source link

Add support for passing traits as argument in contract call #92

Closed friedger closed 3 years ago

friedger commented 4 years ago

Should there be a method like traitCV?

friedger commented 3 years ago

For my marketplace clarity contract (https://github.com/friedger/clarity-marketplace), I am looking for information how to serialize a trait reference type. What needs to be done for traitCV?

psq commented 3 years ago

Maybe I don't understand the question, but if you are wondering how you can call (bid (tradables <tradables-trait>) (tradable-id uint) (price uint)), you would use something like this

const query = client.createQuery({ method: { name: "bid", args: [ "'SP2NC4YKZWM2YMCJV851VF278H9J50ZSNM33P3JM1.tradable", 123, 456] } });

you just pass in a contract principal when a function expects a trait.

friedger commented 3 years ago

@psq There is no client here, the issue is about how to create a transaction for a contract call with a trait as an argument that can then be broadcasted to the network.

If we agree that we should just use the contract principal (constructed with contractPrincipalCV) then traitCV is not needed and the contract principal should be "cast" to a trait when the transaction is received. This does not happen, instead the transaction is rejected with something like:

{ error: 'transaction rejected',
  reason: 'BadFunctionArgument',
  reason_data:
   { message:
      'TypeError(TraitReferenceType(TraitIdentifier { name: ClarityName("tradables-trait"), contract_identifier: QualifiedContractIdentifier { issuer: StandardPrincipalData(26, [210, 137, 135, 159, 210, 247, 227, 61, 194, 243, 163, 67, 70, 194, 6, 243, 93, 231, 54, 150]), name: ContractName("tradables") } }), PrincipalType)' },
  txid:
   'b9bc686ba9289667b85d9ed728250d52989fc5a4e9e7a44048cb298e64bc8817' }
psq commented 3 years ago

I'm not sure why passing in a principal works with clarity-cli, but not when being serialized/deserialized when submitted as a transaction, as internally, it needs to have a TraitReferenceType, not a PrincipalType. Which would mean the simplest would be to implement a traitCV in both stacks-transaction-js and stacks-blockchain, and would probably impact stacks-blockchain-api and maybe connect as well. Something that would need to be discussed with many...

On the other hand, making passing a principal work would make usage simpler, but I'd need to spend some time figuring out how this works in the case of clarity-cli

psq commented 3 years ago

actually, I did not realize that what I had started on would be enough to make it work, but I was also running into the same issue as described in https://github.com/blockstack/stacks-blockchain/issues/1815. Once that other issue is fixed, it should be fairly straightforward to provide a fix.

Running the test with partial fix:

(base) psq-mbp-2:clarity-marketplace psq$ ./node_modules/.bin/ts-node scripts/market.ts 
deploy contract
e5cee627f0fd5e9a008c050754b54359a661040cfbb040ddd6a166a79d6a1e8f
deploy contract
e61a9202a3c947510cfc9d18124815781657b98ccd1dfe35be87cceec9f8b1f8
deploy contract
2acb66c0706632d7d898c948c8b1d97f05ee2405289b673c9077e8bd4fc4546c
deploy contract
329ee7e54b600e6a879de747fea77ffdf5c351d9279a4b2e46260648144a2561
bid for tradable
b5e814a5e4f48f475ad0675f6c20b9ba87635cde9ee6d68cfddeaee3eaca7841
create monster
(node:62367) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
8e554003687eab07481e94ffd9eed2df91e31b15ac82b235779e4d4f4e1059ed
feed monster
1104d18871486c341da2fa58100ced9110407a7854a8413d77083836e68473ee
bid for tradable
11dabc3716f0b6c12bac10c67f6c8d579a6d5f39d2cff2b2daaaf9a4ba9b1a36

So, no need for traitCV...

psq commented 3 years ago

the fix has been merged into next, which will get merged into master for Krypton

diwakergupta commented 3 years ago

the fix has been merged into next, which will get merged into master for Krypton

@psq thanks! Can we close this then or is there any additional work needed in stacks-transactions-js? (if not, we may want to just move this issue out of this repo)

cc/ @agraebe @yknl

psq commented 3 years ago

@diwakergupta yes, this could be closed (I can't), unless you'd rather wait for the fix to be available in a release. The full fix requires both https://github.com/blockstack/stacks-blockchain/issues/1815 and https://github.com/blockstack/stacks-blockchain/issues/1817 (both merged into next now, 1817 already in master)

diwakergupta commented 3 years ago

Thanks, closing!