-
Notifications
You must be signed in to change notification settings - Fork 548
support DynamicNFT #2726
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
support DynamicNFT #2726
Changes from 6 commits
162e6ad
c51ca1c
8d280ce
a22924f
b46debb
4bfc0fa
6ca1b16
f655224
bfd67b4
5e59412
7e3c155
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { ValidationError } from '../../errors' | ||
import { isHex } from '../utils' | ||
|
||
import { | ||
BaseTransaction, | ||
validateBaseTransaction, | ||
isAccount, | ||
isString, | ||
validateOptionalField, | ||
Account, | ||
validateRequiredField, | ||
} from './common' | ||
|
||
/** | ||
* The NFTokenModify transaction modifies an NFToken's URI | ||
* if its tfMutable is set to true. | ||
*/ | ||
export interface NFTokenModify extends BaseTransaction { | ||
TransactionType: 'NFTokenModify' | ||
/** | ||
* Identifies the NFTokenID of the NFToken object that the | ||
* offer references. | ||
*/ | ||
NFTokenID: string | ||
/** | ||
* Indicates the AccountID of the account that owns the | ||
* corresponding NFToken. | ||
yinyiqian1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
Owner?: Account | ||
/** | ||
* URI that points to the data and/or metadata associated with the NFT. | ||
* This field need not be an HTTP or HTTPS URL; it could be an IPFS URI, a | ||
* magnet link, immediate data encoded as an RFC2379 "data" URL, or even an | ||
* opaque issuer-specific encoding. The URI is NOT checked for validity, but | ||
* the field is limited to a maximum length of 256 bytes. | ||
* | ||
* This field must be hex-encoded. You can use `convertStringToHex` to | ||
* convert this field to the proper encoding. | ||
* | ||
* This field must not be an empty string. Omit it from the transaction or | ||
* set to `undefined` value if you do not use it. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could this comment reference setting to null for clarity |
||
*/ | ||
URI?: string | null | ||
} | ||
|
||
/** | ||
* Verify the form and type of an NFTokenModify at runtime. | ||
* | ||
* @param tx - An NFTokenModify Transaction. | ||
* @throws When the NFTokenModify is Malformed. | ||
*/ | ||
export function validateNFTokenModify(tx: Record<string, unknown>): void { | ||
validateBaseTransaction(tx) | ||
|
||
validateRequiredField(tx, 'NFTokenID', isString) | ||
validateOptionalField(tx, 'Owner', isAccount) | ||
validateOptionalField(tx, 'URI', isString) | ||
|
||
if (tx.URI !== undefined && typeof tx.URI === 'string') { | ||
if (tx.URI === '') { | ||
throw new ValidationError('NFTokenModify: URI must not be empty string') | ||
} | ||
yinyiqian1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!isHex(tx.URI)) { | ||
throw new ValidationError('NFTokenModify: URI must be in hex format') | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
import { assert } from 'chai' | ||
|
||
import { NFTokenModify } from '../../../dist/npm' | ||
import { NFTokenMintFlags } from '../../../dist/npm/src' | ||
import { | ||
convertStringToHex, | ||
getNFTokenID, | ||
NFTokenMint, | ||
TransactionMetadata, | ||
TxRequest, | ||
} from '../../../src' | ||
import { hashSignedTx } from '../../../src/utils/hashes' | ||
import serverUrl from '../serverUrl' | ||
import { | ||
setupClient, | ||
teardownClient, | ||
type XrplIntegrationTestContext, | ||
} from '../setup' | ||
import { testTransaction } from '../utils' | ||
|
||
// how long before each test case times out | ||
const TIMEOUT = 20000 | ||
|
||
describe('NFTokenModify', function () { | ||
let testContext: XrplIntegrationTestContext | ||
|
||
beforeEach(async () => { | ||
testContext = await setupClient(serverUrl) | ||
}) | ||
afterEach(async () => teardownClient(testContext)) | ||
|
||
// Mint an NFToken with tfMutable flag and modify URI later | ||
it( | ||
'modify NFToken URI', | ||
async function () { | ||
const oldUri = convertStringToHex('https://www.google.com') | ||
const newUri = convertStringToHex('https://www.ripple.com') | ||
|
||
const mutableMint: NFTokenMint = { | ||
TransactionType: 'NFTokenMint', | ||
Account: testContext.wallet.address, | ||
Flags: NFTokenMintFlags.tfMutable, | ||
URI: oldUri, | ||
NFTokenTaxon: 0, | ||
} | ||
const response = await testTransaction( | ||
testContext.client, | ||
mutableMint, | ||
testContext.wallet, | ||
) | ||
assert.equal(response.type, 'response') | ||
|
||
const mutableTx: TxRequest = { | ||
command: 'tx', | ||
transaction: hashSignedTx(response.result.tx_blob), | ||
} | ||
const mutableTxResponse = await testContext.client.request(mutableTx) | ||
Comment on lines
+53
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how come we need a separate tx request here? could we get the nft id from the mint response There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mint response won't provide nftoken id. we have to query it through another request |
||
|
||
const mutableNFTokenID = | ||
getNFTokenID( | ||
mutableTxResponse.result.meta as TransactionMetadata<NFTokenMint>, | ||
) ?? 'undefined' | ||
|
||
const accountNFTs = await testContext.client.request({ | ||
command: 'account_nfts', | ||
account: testContext.wallet.address, | ||
}) | ||
|
||
assert.equal( | ||
accountNFTs.result.account_nfts.find( | ||
(nft) => nft.NFTokenID === mutableNFTokenID, | ||
)?.URI, | ||
oldUri, | ||
) | ||
|
||
const modifyTx: NFTokenModify = { | ||
TransactionType: 'NFTokenModify', | ||
Account: testContext.wallet.address, | ||
NFTokenID: mutableNFTokenID, | ||
URI: newUri, | ||
} | ||
|
||
const modifyResponse = await testTransaction( | ||
testContext.client, | ||
modifyTx, | ||
testContext.wallet, | ||
) | ||
assert.equal(modifyResponse.type, 'response') | ||
|
||
const nfts = await testContext.client.request({ | ||
command: 'account_nfts', | ||
account: testContext.wallet.address, | ||
}) | ||
|
||
assert.equal( | ||
nfts.result.account_nfts.find( | ||
(nft) => nft.NFTokenID === mutableNFTokenID, | ||
)?.URI, | ||
newUri, | ||
) | ||
}, | ||
TIMEOUT, | ||
) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import { assert } from 'chai' | ||
|
||
import { convertStringToHex, validate, ValidationError } from '../../src' | ||
|
||
const TOKEN_ID = | ||
'00090032B5F762798A53D543A014CAF8B297CFF8F2F937E844B17C9E00000003' | ||
|
||
/** | ||
* NFTokenModify Transaction Verification Testing. | ||
* | ||
* Providing runtime verification testing for each specific transaction type. | ||
*/ | ||
describe('NFTokenModify', function () { | ||
it(`verifies valid NFTokenModify`, function () { | ||
const validNFTokenModify = { | ||
TransactionType: 'NFTokenModify', | ||
Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', | ||
NFTokenID: TOKEN_ID, | ||
Fee: '5000000', | ||
Sequence: 2470665, | ||
URI: convertStringToHex('http://xrpl.org'), | ||
} as any | ||
|
||
assert.doesNotThrow(() => validate(validNFTokenModify)) | ||
}) | ||
|
||
it(`throws w/ missing NFTokenID`, function () { | ||
const invalid = { | ||
TransactionType: 'NFTokenModify', | ||
Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm', | ||
Fee: '5000000', | ||
Sequence: 2470665, | ||
} as any | ||
|
||
assert.throws( | ||
() => validate(invalid), | ||
ValidationError, | ||
'NFTokenModify: missing field NFTokenID', | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you add test cases for invalid hex format and empty string check here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a separate PR to add the test and resolve comments: #2892 |
||
}) | ||
}) |
Uh oh!
There was an error while loading. Please reload this page.
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.
nit: can follow the format on line 13 and indicate XLS-46 instead of adding the specifics here