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

Hotfix BRC20 assertion contents when data is empty #2649

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

zhouhuitian
Copy link
Contributor

Regarding the case of an empty response, there are two modifications:

  1. When the response data is empty, the default display will be these seven tokens we supported currently. ["ordi", "sats", "rats", "MMSS", "long", "cats", "BTCs"] , with all balances set to 0.0. The effect is as shown below.
{
    "assertions": [
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$ordi"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "1"
                }
            ]
        },
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$sats"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "40000000"
                }
            ]
        },
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$rats"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "40000"
                }
            ]
        },
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$MMSS"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "20"
                }
            ]
        },
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$long"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "20"
                }
            ]
        },
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$cats"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "10000"
                }
            ]
        },
        {
            "and": [
                {
                    "src": "$token",
                    "op": "==",
                    "dst": "$BTCs"
                },
                {
                    "src": "$holding_amount",
                    "op": ">=",
                    "dst": "0"
                },
                {
                    "src": "$holding_amount",
                    "op": "<",
                    "dst": "5"
                }
            ]
        }
    ]
}
  1. The "value" will always be true, because regardless of the balance amount, it will always fall within a certain range.

@zhouhuitian zhouhuitian requested review from jonalvarezz, BillyWooo and a team April 10, 2024 13:09
@zhouhuitian zhouhuitian added the C0-breaking Breaking change that will cause existing client to break label Apr 10, 2024
Copy link
Contributor

@jonalvarezz jonalvarezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of comments but looking good to me overall.

btw, the breaking-change may not be needed, at least not for clients.

Comment on lines 29 to 30
// Keep all name in lowercase here by purpose
const BRC20_TOKENS: [&str; 7] = ["ordi", "sats", "rats", "mmss", "long", "cats", "btcs"];
const BRC20_TOKENS: [&str; 7] = ["ordi", "sats", "rats", "MMSS", "long", "cats", "BTCs"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we remove the comment on line 29 or are we missing something?


response_items
.iter()
.filter(|&item| BRC20_TOKENS.contains(&item.tick.as_str()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we compare using lowercases to forget about it? e.g., data provider changes the case for cats -> CATs

@@ -72,30 +79,60 @@ pub trait BRC20AmountHolderCredential {

impl BRC20AmountHolderCredential for Credential {
fn update_brc20_amount_holder_credential(&mut self, response_items: &[ResponseItem]) {
for item in response_items {
if BRC20_TOKENS.contains(&item.tick.to_lowercase().as_str()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we keep to_lowercase() for simplicity? then the above BRC20_TOEKN doesn't need to change.

@zhouhuitian zhouhuitian removed the C0-breaking Breaking change that will cause existing client to break label Apr 11, 2024
@zhouhuitian
Copy link
Contributor Author

Ok, keep BRC20_TOKENS letters in lowercase.

@zhouhuitian zhouhuitian changed the title Update BRC20 assertion contents when data is empty Hotfix BRC20 assertion contents when data is empty Apr 11, 2024
@BillyWooo BillyWooo merged commit f5e2c33 into dev Apr 12, 2024
26 checks passed
@BillyWooo BillyWooo deleted the fix-brc20-none-assertions branch April 12, 2024 10:56
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

Successfully merging this pull request may close these issues.

None yet

3 participants