feat(cosmos): Add support for contract call transaction #3691
Conversation
dpkjnr
force-pushed
the
cosmos-contract-txn
branch
6 times, most recently
from
a67fd14
to
9f5715a
Compare
| private registry; | ||
| constructor() { | ||
| this.registry = new Registry([...defaultRegistryTypes]); | ||
| this.registry.register('/cosmwasm.wasm.v1.MsgExecuteContract', MsgExecuteContract); |
There was a problem hiding this comment.
nit: could be a constant
There was a problem hiding this comment.
It should already be there in constants, let me use that here
| export const accountAddressRegex = /^(cosmos)1(['qpzry9x8gf2tvdw0s3jn54khce6mua7l']{38})$/; | ||
| export const validatorAddressRegex = /^(cosmosvaloper)1(['qpzry9x8gf2tvdw0s3jn54khce6mua7l']{38})$/; |
There was a problem hiding this comment.
What changed here ?
There was a problem hiding this comment.
length, for account address it would always be 38 character long(excluding prefix)
| isValidContractAddress(address: string): boolean { | ||
| return constants.contractAddressRegex.test(address); | ||
| } |
There was a problem hiding this comment.
this and few other methods - seems like we could put it together in a parent class after abstracting the regex pattern; not too concerned though
There was a problem hiding this comment.
I was also thinking to do that, but it seemed more clean to me, let me check it again
There was a problem hiding this comment.
I think it can be done but I don't want to delay this PR, will create a separate PR for it after completing wallet platform changes
| @@ -129,7 +129,7 @@ export class CosmosCoin extends BaseCoin { | |||
| throw new Error('Tx outputs does not match with expected txParams recipients'); | |||
| } | |||
| // WithdrawDelegatorRewards transaction doesn't have amount | |||
| if (transaction.type !== TransactionType.StakingWithdraw) { | |||
| if (transaction.type !== TransactionType.StakingWithdraw && transaction.type !== TransactionType.ContractCall) { | |||
There was a problem hiding this comment.
Contract interaction support the amount, anyway that will not require in our case but let's keep in mind.
There was a problem hiding this comment.
yeah, it is under funds and didn't want to update current amount validation for it
| * @returns {boolean} - the validation result | ||
| */ | ||
| isValidContractAddress(address: string): boolean { | ||
| throw new NotImplementedError('isValidContractAddress not implemented'); |
There was a problem hiding this comment.
let's implement using the length comparison and add ToDo to implement with Regex. Else it will be blocker in the platform side.
There was a problem hiding this comment.
it is there in extended classes
dpkjnr
force-pushed
the
cosmos-contract-txn
branch
from
9f5715a
to
9b7dd54
Compare
|
|
||
| import * as crypto from 'crypto'; | ||
| import * as constants from './constants'; | ||
| import { | ||
| CosmosLikeTransaction, | ||
| DelegateOrUndelegeteMessage, | ||
| ExecuteContractMessage, |
There was a problem hiding this comment.
Can we use CosmosTransactionMessage here.
There was a problem hiding this comment.
we can't, schema is different
| @@ -473,19 +501,23 @@ export class CosmosUtils implements BaseUtils { | |||
| switch (type) { | |||
| case TransactionType.Send: { | |||
| const value = messageData.value as SendMessage; | |||
| this.isObjPropertyNull(value, ['toAddress', 'fromAddress']); | |||
There was a problem hiding this comment.
why are we removing this null check ??
There was a problem hiding this comment.
it is being done in this.validateSendMessage, I wanted to keep validation code common
There was a problem hiding this comment.
Can you check this function's path - enrichTransactionDetailsFromRawTransaction()
I think we wont be making this null check here. As this is not calling this.validateSendMessage.
| break; | ||
| } | ||
| case TransactionType.StakingWithdraw: { | ||
| const value = messageData.value as WithdrawDelegatorRewardsMessage; | ||
| this.isObjPropertyNull(value, ['validatorAddress', 'delegatorAddress']); |
Ticket: WIN-20